Fix integer-overflow edge case detection in interval_mul and pgbench.

This patch adopts the overflow check logic introduced by commit cbdb8b4c0
into two more places.  interval_mul() failed to notice if it computed a
new microseconds value that was one more than INT64_MAX, and pgbench's
double-to-int64 logic had the same sorts of edge-case problems that
cbdb8b4c0 fixed in the core code.

To make this easier to get right in future, put the guts of the checks
into new macros in c.h, and add commentary about how to use the macros
correctly.

Back-patch to all supported branches, as we did with the previous fix.

Yuya Watari

Discussion: https://postgr.es/m/CAJ2pMkbkkFw2hb9Qb1Zj8d06EhWAQXFLy73St4qWv6aX=vqnjw@mail.gmail.com

diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index 5ccfdeab11cb89bd48e7e3080b73fafa42cdad56..89e40ecac9848943e99d8a0e0db15e03ab6ffa0d 100644 (file)
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -1227,15 +1227,8 @@ dtoi4(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT32_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT32_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float8) PG_INT32_MIN ||
-               num >= -((float8) PG_INT32_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT8_FITS_IN_INT32(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("integer out of range")));
@@ -1259,15 +1252,8 @@ dtoi2(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT16_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT16_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float8) PG_INT16_MIN ||
-               num >= -((float8) PG_INT16_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT8_FITS_IN_INT16(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("smallint out of range")));
@@ -1315,15 +1301,8 @@ ftoi4(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT32_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT32_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float4) PG_INT32_MIN ||
-               num >= -((float4) PG_INT32_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT4_FITS_IN_INT32(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("integer out of range")));
@@ -1347,15 +1326,8 @@ ftoi2(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT16_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT16_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float4) PG_INT16_MIN ||
-               num >= -((float4) PG_INT16_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT4_FITS_IN_INT16(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("smallint out of range")));

diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 54135414d5681486cb3853a54ae956c701755b80..d3efd8af20c6069dab5c61aef6f45b15107afbe4 100644 (file)
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -1351,15 +1351,8 @@ dtoi8(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT64_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT64_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float8) PG_INT64_MIN ||
-               num >= -((float8) PG_INT64_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT8_FITS_IN_INT64(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("bigint out of range")));
@@ -1393,15 +1386,8 @@ ftoi8(PG_FUNCTION_ARGS)
         */
        num = rint(num);
 
-       /*
-        * Range check.  We must be careful here that the boundary values are
-        * expressed exactly in the float domain.  We expect PG_INT64_MIN to be an
-        * exact power of 2, so it will be represented exactly; but PG_INT64_MAX
-        * isn't, and might get rounded off, so avoid using it.
-        */
-       if (num < (float4) PG_INT64_MIN ||
-               num >= -((float4) PG_INT64_MIN) ||
-               isnan(num))
+       /* Range check */
+       if (isnan(num) || !FLOAT4_FITS_IN_INT64(num))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("bigint out of range")));

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 64463f1271d8cfaf97f58a9cea1a6f34897df05f..100c8c1cb3906e7107be24b4c98ab727959686e1 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3181,7 +3181,7 @@ interval_mul(PG_FUNCTION_ARGS)
        /* cascade units down */
        result->day += (int32) month_remainder_days;
        result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
-       if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN)
+       if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
                ereport(ERROR,
                                (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
                                 errmsg("interval out of range")));

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 341dc8bba9b34fe91857fa9d1a3f89f69d9cb791..247f1e1eaddaf7ea1846cb2cac9582e49dd5a23c 100644 (file)
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1236,10 +1236,11 @@ coerceToInt(PgBenchValue *pval, int64 *ival)
        }
        else
        {
-               double          dval = pval->u.dval;
+               double          dval;
 
                Assert(pval->type == PGBT_DOUBLE);
-               if (dval < PG_INT64_MIN || PG_INT64_MAX < dval)
+               dval = rint(pval->u.dval);
+               if (isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))
                {
                        fprintf(stderr, "double to int overflow for %f\n", dval);
                        return false;

diff --git a/src/include/c.h b/src/include/c.h
index f6db6754df323c6f8fb19242fa4f01e9a3c1c8d5..9d91401aba52119c8e5993037643b9be440f3157 100644 (file)
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -963,6 +963,30 @@ typedef NameData *Name;
                        *_start++ = 0; \
        } while (0)
 
+/*
+ * Macros for range-checking float values before converting to integer.
+ * We must be careful here that the boundary values are expressed exactly
+ * in the float domain.  PG_INTnn_MIN is an exact power of 2, so it will
+ * be represented exactly; but PG_INTnn_MAX isn't, and might get rounded
+ * off, so avoid using that.
+ * The input must be rounded to an integer beforehand, typically with rint(),
+ * else we might draw the wrong conclusion about close-to-the-limit values.
+ * These macros will do the right thing for Inf, but not necessarily for NaN,
+ * so check isnan(num) first if that's a possibility.
+ */
+#define FLOAT4_FITS_IN_INT16(num) \
+       ((num) >= (float4) PG_INT16_MIN && (num) < -((float4) PG_INT16_MIN))
+#define FLOAT4_FITS_IN_INT32(num) \
+       ((num) >= (float4) PG_INT32_MIN && (num) < -((float4) PG_INT32_MIN))
+#define FLOAT4_FITS_IN_INT64(num) \
+       ((num) >= (float4) PG_INT64_MIN && (num) < -((float4) PG_INT64_MIN))
+#define FLOAT8_FITS_IN_INT16(num) \
+       ((num) >= (float8) PG_INT16_MIN && (num) < -((float8) PG_INT16_MIN))
+#define FLOAT8_FITS_IN_INT32(num) \
+       ((num) >= (float8) PG_INT32_MIN && (num) < -((float8) PG_INT32_MIN))
+#define FLOAT8_FITS_IN_INT64(num) \
+       ((num) >= (float8) PG_INT64_MIN && (num) < -((float8) PG_INT64_MIN))
+
 
 /* ----------------------------------------------------------------
  *                             Section 8:      random stuff

diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index f88f34550ad56b44d37785a772e329a87c0eedb9..f772909e49ce949829fd15c80347c871f65cbc73 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -232,6 +232,9 @@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years');
 ERROR:  interval out of range
 LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'...
                                                  ^
+-- Test edge-case overflow detection in interval multiplication
+select extract(epoch from '256 microseconds'::interval * (2^55)::float8);
+ERROR:  interval out of range
 SELECT r1.*, r2.*
    FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2
    WHERE r1.f1 > r2.f1

diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index bc5537d1b9c008dc9b63e732b6b31b338880f8af..eb1e84f053e05ddc6d4448ec59a41911a1a8ed1e 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -73,6 +73,9 @@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days');
 INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years');
 INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years');
 
+-- Test edge-case overflow detection in interval multiplication
+select extract(epoch from '256 microseconds'::interval * (2^55)::float8);
+
 SELECT r1.*, r2.*
    FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2
    WHERE r1.f1 > r2.f1