Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
projects / postgresql.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 53c2a97)
Fix mis-rounding and overflow hazards in date_bin().
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 28 Feb 2024 19:00:30 +0000 (14:00 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 28 Feb 2024 19:00:30 +0000 (14:00 -0500)
In the case where the target timestamp is before the origin timestamp
and their difference is already an exact multiple of the stride, the
code incorrectly subtracted the stride anyway.

Also detect several integer-overflow cases that previously produced
bogus results.  (The submitted patch tried to avoid overflow, but
I'm not convinced it's right, and problematic cases are so far out of
the plausibly-useful range that they don't seem worth sweating over.
Let's just use overflow-detecting arithmetic and throw errors.)

timestamp_bin() and timestamptz_bin() are basically identical and
so had identical bugs.  Fix both.

Report and patch by Moaaz Assali, adjusted some by me.  Back-patch
to v14 where date_bin() was introduced.

Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com

src/backend/utils/adt/timestamp.c patch | blob | blame | history
src/test/regress/expected/timestamp.out patch | blob | blame | history
src/test/regress/expected/timestamptz.out patch | blob | blame | history
src/test/regress/sql/timestamp.sql patch | blob | blame | history
src/test/regress/sql/timestamptz.sql patch | blob | blame | history

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index ed03c50a6dead2d5c6e27d68c76c3199cdfae927..7a016a69234afae7a325b19ae45d2a98f95006c1 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -4539,8 +4539,9 @@ timestamp_bin(PG_FUNCTION_ARGS)
    Timestamp   timestamp = PG_GETARG_TIMESTAMP(1);
    Timestamp   origin = PG_GETARG_TIMESTAMP(2);
    Timestamp   result,
-               tm_diff,
                stride_usecs,
+               tm_diff,
+               tm_modulo,
                tm_delta;
 
    if (TIMESTAMP_NOT_FINITE(timestamp))
@@ -4561,24 +4562,40 @@ timestamp_bin(PG_FUNCTION_ARGS)
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("timestamps cannot be binned into intervals containing months or years")));
 
-   stride_usecs = stride->day * USECS_PER_DAY + stride->time;
+   if (unlikely(pg_mul_s64_overflow(stride->day, USECS_PER_DAY, &stride_usecs)) ||
+       unlikely(pg_add_s64_overflow(stride_usecs, stride->time, &stride_usecs)))
+       ereport(ERROR,
+               (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                errmsg("interval out of range")));
 
    if (stride_usecs <= 0)
        ereport(ERROR,
                (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
                 errmsg("stride must be greater than zero")));
 
-   tm_diff = timestamp - origin;
-   tm_delta = tm_diff - tm_diff % stride_usecs;
+   if (unlikely(pg_sub_s64_overflow(timestamp, origin, &tm_diff)))
+       ereport(ERROR,
+               (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                errmsg("interval out of range")));
+
+   /* These calculations cannot overflow */
+   tm_modulo = tm_diff % stride_usecs;
+   tm_delta = tm_diff - tm_modulo;
+   result = origin + tm_delta;
 
    /*
-    * Make sure the returned timestamp is at the start of the bin, even if
-    * the origin is in the future.
+    * We want to round towards -infinity, not 0, when tm_diff is negative and
+    * not a multiple of stride_usecs.  This adjustment *can* cause overflow,
+    * since the result might now be out of the range origin .. timestamp.
     */
-   if (origin > timestamp && stride_usecs > 1)
-       tm_delta -= stride_usecs;
-
-   result = origin + tm_delta;
+   if (tm_modulo < 0)
+   {
+       if (unlikely(pg_sub_s64_overflow(result, stride_usecs, &result)) ||
+           !IS_VALID_TIMESTAMP(result))
+           ereport(ERROR,
+                   (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                    errmsg("timestamp out of range")));
+   }
 
    PG_RETURN_TIMESTAMP(result);
 }
@@ -4729,6 +4746,7 @@ timestamptz_bin(PG_FUNCTION_ARGS)
    TimestampTz result,
                stride_usecs,
                tm_diff,
+               tm_modulo,
                tm_delta;
 
    if (TIMESTAMP_NOT_FINITE(timestamp))
@@ -4749,24 +4767,40 @@ timestamptz_bin(PG_FUNCTION_ARGS)
                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("timestamps cannot be binned into intervals containing months or years")));
 
-   stride_usecs = stride->day * USECS_PER_DAY + stride->time;
+   if (unlikely(pg_mul_s64_overflow(stride->day, USECS_PER_DAY, &stride_usecs)) ||
+       unlikely(pg_add_s64_overflow(stride_usecs, stride->time, &stride_usecs)))
+       ereport(ERROR,
+               (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                errmsg("interval out of range")));
 
    if (stride_usecs <= 0)
        ereport(ERROR,
                (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
                 errmsg("stride must be greater than zero")));
 
-   tm_diff = timestamp - origin;
-   tm_delta = tm_diff - tm_diff % stride_usecs;
+   if (unlikely(pg_sub_s64_overflow(timestamp, origin, &tm_diff)))
+       ereport(ERROR,
+               (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                errmsg("interval out of range")));
+
+   /* These calculations cannot overflow */
+   tm_modulo = tm_diff % stride_usecs;
+   tm_delta = tm_diff - tm_modulo;
+   result = origin + tm_delta;
 
    /*
-    * Make sure the returned timestamp is at the start of the bin, even if
-    * the origin is in the future.
+    * We want to round towards -infinity, not 0, when tm_diff is negative and
+    * not a multiple of stride_usecs.  This adjustment *can* cause overflow,
+    * since the result might now be out of the range origin .. timestamp.
     */
-   if (origin > timestamp && stride_usecs > 1)
-       tm_delta -= stride_usecs;
-
-   result = origin + tm_delta;
+   if (tm_modulo < 0)
+   {
+       if (unlikely(pg_sub_s64_overflow(result, stride_usecs, &result)) ||
+           !IS_VALID_TIMESTAMP(result))
+           ereport(ERROR,
+                   (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+                    errmsg("timestamp out of range")));
+   }
 
    PG_RETURN_TIMESTAMPTZ(result);
 }
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index 835f0e5762fa05ec676c2d1d211a6edc35d1c2e9..cf337da517ef274e838820484521491018d66c75 100644 (file)
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -736,6 +736,13 @@ SELECT date_bin('5 min'::interval, timestamp '2020-02-01 01:01:01', timestamp '2
  Sat Feb 01 00:57:30 2020
 (1 row)
 
+-- test roundoff edge case when source < origin
+SELECT date_bin('30 minutes'::interval, timestamp '2024-02-01 15:00:00', timestamp '2024-02-01 17:00:00');
+         date_bin         
+--------------------------
+ Thu Feb 01 15:00:00 2024
+(1 row)
+
 -- disallow intervals with months or years
 SELECT date_bin('5 months'::interval, timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
 ERROR:  timestamps cannot be binned into intervals containing months or years
@@ -747,6 +754,13 @@ ERROR:  stride must be greater than zero
 -- disallow negative intervals
 SELECT date_bin('-2 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp '1970-01-01 00:00:00');
 ERROR:  stride must be greater than zero
+-- test overflow cases
+select date_bin('15 minutes'::interval, timestamp '294276-12-30', timestamp '4000-12-20 BC');
+ERROR:  interval out of range
+select date_bin('200000000 days'::interval, '2024-02-01'::timestamp, '2024-01-01'::timestamp);
+ERROR:  interval out of range
+select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamp, '4000-01-01 BC'::timestamp);
+ERROR:  timestamp out of range
 -- Test casting within a BETWEEN qualifier
 SELECT d1 - timestamp without time zone '1997-01-02' AS diff
   FROM TIMESTAMP_TBL
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index a084357480f4031d97de3af8cf7cecd7adf1cb6c..bfb3825ff6cf0f99288e7ef549f9f3a08f73ca3c 100644 (file)
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -780,6 +780,13 @@ SELECT date_bin('5 min'::interval, timestamptz '2020-02-01 01:01:01+00', timesta
  Fri Jan 31 16:57:30 2020 PST
 (1 row)
 
+-- test roundoff edge case when source < origin
+SELECT date_bin('30 minutes'::interval, timestamptz '2024-02-01 15:00:00', timestamptz '2024-02-01 17:00:00');
+           date_bin           
+------------------------------
+ Thu Feb 01 15:00:00 2024 PST
+(1 row)
+
 -- disallow intervals with months or years
 SELECT date_bin('5 months'::interval, timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
 ERROR:  timestamps cannot be binned into intervals containing months or years
@@ -791,6 +798,13 @@ ERROR:  stride must be greater than zero
 -- disallow negative intervals
 SELECT date_bin('-2 days'::interval, timestamp with time zone '1970-01-01 01:00:00+00' , timestamp with time zone '1970-01-01 00:00:00+00');
 ERROR:  stride must be greater than zero
+-- test overflow cases
+select date_bin('15 minutes'::interval, timestamptz '294276-12-30', timestamptz '4000-12-20 BC');
+ERROR:  interval out of range
+select date_bin('200000000 days'::interval, '2024-02-01'::timestamptz, '2024-01-01'::timestamptz);
+ERROR:  interval out of range
+select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamptz, '4000-01-01 BC'::timestamptz);
+ERROR:  timestamp out of range
 -- Test casting within a BETWEEN qualifier
 SELECT d1 - timestamp with time zone '1997-01-02' AS diff
   FROM TIMESTAMPTZ_TBL
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index ea12ffd18d458a85c06a07319de9dde2134f4bce..820ef7752ac7cb569465a86a7c10950fc9f64d21 100644 (file)
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -268,6 +268,9 @@ FROM (
 -- shift bins using the origin parameter:
 SELECT date_bin('5 min'::interval, timestamp '2020-02-01 01:01:01', timestamp '2020-02-01 00:02:30');
 
+-- test roundoff edge case when source < origin
+SELECT date_bin('30 minutes'::interval, timestamp '2024-02-01 15:00:00', timestamp '2024-02-01 17:00:00');
+
 -- disallow intervals with months or years
 SELECT date_bin('5 months'::interval, timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
 SELECT date_bin('5 years'::interval,  timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
@@ -278,6 +281,11 @@ SELECT date_bin('0 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp
 -- disallow negative intervals
 SELECT date_bin('-2 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp '1970-01-01 00:00:00');
 
+-- test overflow cases
+select date_bin('15 minutes'::interval, timestamp '294276-12-30', timestamp '4000-12-20 BC');
+select date_bin('200000000 days'::interval, '2024-02-01'::timestamp, '2024-01-01'::timestamp);
+select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamp, '4000-01-01 BC'::timestamp);
+
 -- Test casting within a BETWEEN qualifier
 SELECT d1 - timestamp without time zone '1997-01-02' AS diff
   FROM TIMESTAMP_TBL
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index a2dcd5f5d8e2f711d20524fb30eebbe80ebe2146..ccfd90d6467ae985006e4ed19d39a6a6be44c82d 100644 (file)
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -243,6 +243,9 @@ FROM (
 -- shift bins using the origin parameter:
 SELECT date_bin('5 min'::interval, timestamptz '2020-02-01 01:01:01+00', timestamptz '2020-02-01 00:02:30+00');
 
+-- test roundoff edge case when source < origin
+SELECT date_bin('30 minutes'::interval, timestamptz '2024-02-01 15:00:00', timestamptz '2024-02-01 17:00:00');
+
 -- disallow intervals with months or years
 SELECT date_bin('5 months'::interval, timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
 SELECT date_bin('5 years'::interval,  timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
@@ -253,6 +256,11 @@ SELECT date_bin('0 days'::interval, timestamp with time zone '1970-01-01 01:00:0
 -- disallow negative intervals
 SELECT date_bin('-2 days'::interval, timestamp with time zone '1970-01-01 01:00:00+00' , timestamp with time zone '1970-01-01 00:00:00+00');
 
+-- test overflow cases
+select date_bin('15 minutes'::interval, timestamptz '294276-12-30', timestamptz '4000-12-20 BC');
+select date_bin('200000000 days'::interval, '2024-02-01'::timestamptz, '2024-01-01'::timestamptz);
+select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamptz, '4000-01-01 BC'::timestamptz);
+
 -- Test casting within a BETWEEN qualifier
 SELECT d1 - timestamp with time zone '1997-01-02' AS diff
   FROM TIMESTAMPTZ_TBL
This is the main PostgreSQL git repository.