]> granicus.if.org Git - postgresql/commitdiff
Fix float-to-integer coercions to handle edge cases correctly.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 24 Nov 2018 17:45:50 +0000 (12:45 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 24 Nov 2018 17:45:50 +0000 (12:45 -0500)
ftoi4 and its sibling coercion functions did their overflow checks in
a way that looked superficially plausible, but actually depended on an
assumption that the MIN and MAX comparison constants can be represented
exactly in the float4 or float8 domain.  That fails in ftoi4, ftoi8,
and dtoi8, resulting in a possibility that values near the MAX limit will
be wrongly converted (to negative values) when they need to be rejected.

Also, because we compared before rounding off the fractional part,
the other three functions threw errors for values that really ought
to get rounded to the min or max integer value.

Fix by doing rint() first (requiring an assumption that it handles
NaN and Inf correctly; but dtoi8 and ftoi8 were assuming that already),
and by comparing to values that should coerce to float exactly, namely
INTxx_MIN and -INTxx_MIN.  Also remove some random cosmetic discrepancies
between these six functions.

This back-patches commits cbdb8b4c0 and 452b637d4.  In the 9.4 branch,
also back-patch the portion of 62e2a8dc2 that added PG_INTnn_MIN and
related constants to c.h, so that these functions can rely on them.

Per bug #15519 from Victor Petrovykh.

Patch by me; thanks to Andrew Gierth for analysis and discussion.

Discussion: https://postgr.es/m/15519-4fc785b483201ff1@postgresql.org

src/backend/utils/adt/float.c
src/backend/utils/adt/int8.c
src/include/c.h
src/test/regress/expected/float4.out
src/test/regress/expected/float8-small-is-zero.out
src/test/regress/expected/float8.out
src/test/regress/sql/float4.sql
src/test/regress/sql/float8.sql

index 0b4637f100409842b8713000c9a1edde8c7fe736..00ae55106dd9cfddeec27db9bb08be719bde4d07 100644 (file)
@@ -1149,16 +1149,28 @@ Datum
 dtoi4(PG_FUNCTION_ARGS)
 {
        float8          num = PG_GETARG_FLOAT8(0);
-       int32           result;
 
-       /* 'Inf' is handled by INT_MAX */
-       if (num < INT_MIN || num > INT_MAX || isnan(num))
+       /*
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
+        */
+       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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("integer out of range")));
 
-       result = (int32) rint(num);
-       PG_RETURN_INT32(result);
+       PG_RETURN_INT32((int32) num);
 }
 
 
@@ -1170,12 +1182,27 @@ dtoi2(PG_FUNCTION_ARGS)
 {
        float8          num = PG_GETARG_FLOAT8(0);
 
-       if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
+       /*
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
+        */
+       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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("smallint out of range")));
 
-       PG_RETURN_INT16((int16) rint(num));
+       PG_RETURN_INT16((int16) num);
 }
 
 
@@ -1211,12 +1238,27 @@ ftoi4(PG_FUNCTION_ARGS)
 {
        float4          num = PG_GETARG_FLOAT4(0);
 
-       if (num < INT_MIN || num > INT_MAX || isnan(num))
+       /*
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
+        */
+       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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("integer out of range")));
 
-       PG_RETURN_INT32((int32) rint(num));
+       PG_RETURN_INT32((int32) num);
 }
 
 
@@ -1228,12 +1270,27 @@ ftoi2(PG_FUNCTION_ARGS)
 {
        float4          num = PG_GETARG_FLOAT4(0);
 
-       if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
+       /*
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
+        */
+       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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("smallint out of range")));
 
-       PG_RETURN_INT16((int16) rint(num));
+       PG_RETURN_INT16((int16) num);
 }
 
 
index 96146e0fda0c9123f5210d9f4deca3840e3ae81a..ade9b81386df67a5250d1914ba6a7ff959404e8c 100644 (file)
@@ -1342,25 +1342,29 @@ i8tod(PG_FUNCTION_ARGS)
 Datum
 dtoi8(PG_FUNCTION_ARGS)
 {
-       float8          arg = PG_GETARG_FLOAT8(0);
-       int64           result;
-
-       /* Round arg to nearest integer (but it's still in float form) */
-       arg = rint(arg);
+       float8          num = PG_GETARG_FLOAT8(0);
 
        /*
-        * Does it fit in an int64?  Avoid assuming that we have handy constants
-        * defined for the range boundaries, instead test for overflow by
-        * reverse-conversion.
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
         */
-       result = (int64) arg;
+       num = rint(num);
 
-       if ((float8) result != arg)
+       /*
+        * 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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("bigint out of range")));
 
-       PG_RETURN_INT64(result);
+       PG_RETURN_INT64((int64) num);
 }
 
 Datum
@@ -1380,26 +1384,29 @@ i8tof(PG_FUNCTION_ARGS)
 Datum
 ftoi8(PG_FUNCTION_ARGS)
 {
-       float4          arg = PG_GETARG_FLOAT4(0);
-       int64           result;
-       float8          darg;
-
-       /* Round arg to nearest integer (but it's still in float form) */
-       darg = rint(arg);
+       float4          num = PG_GETARG_FLOAT4(0);
 
        /*
-        * Does it fit in an int64?  Avoid assuming that we have handy constants
-        * defined for the range boundaries, instead test for overflow by
-        * reverse-conversion.
+        * Get rid of any fractional part in the input.  This is so we don't fail
+        * on just-out-of-range values that would round into range.  Note
+        * assumption that rint() will pass through a NaN or Inf unchanged.
         */
-       result = (int64) darg;
+       num = rint(num);
 
-       if ((float8) result != darg)
+       /*
+        * 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))
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("bigint out of range")));
 
-       PG_RETURN_INT64(result);
+       PG_RETURN_INT64((int64) num);
 }
 
 Datum
index aad18a7c65149678b851c2913361859f0faa5a66..cb369692e6a7e5d076b05ff9dbc4781071fd7bb5 100644 (file)
@@ -283,6 +283,23 @@ typedef unsigned long long int uint64;
 #error must have a working 64-bit integer datatype
 #endif
 
+/*
+ * stdint.h limits aren't guaranteed to be present and aren't guaranteed to
+ * have compatible types with our fixed width types. So just define our own.
+ */
+#define PG_INT8_MIN            (-0x7F-1)
+#define PG_INT8_MAX            (0x7F)
+#define PG_UINT8_MAX   (0xFF)
+#define PG_INT16_MIN   (-0x7FFF-1)
+#define PG_INT16_MAX   (0x7FFF)
+#define PG_UINT16_MAX  (0xFFFF)
+#define PG_INT32_MIN   (-0x7FFFFFFF-1)
+#define PG_INT32_MAX   (0x7FFFFFFF)
+#define PG_UINT32_MAX  (0xFFFFFFFFU)
+#define PG_INT64_MIN   (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
+#define PG_INT64_MAX   INT64CONST(0x7FFFFFFFFFFFFFFF)
+#define PG_UINT64_MAX  UINT64CONST(0xFFFFFFFFFFFFFFFF)
+
 /* Max value of size_t might be missing if we don't have stdint.h */
 #ifndef SIZE_MAX
 #if SIZEOF_SIZE_T == 8
index fd46a4a1db70da35bac51236c0b76d5f5514b189..2f47e1c202a9d6f3369d225434240f6bb501fab8 100644 (file)
@@ -257,3 +257,52 @@ SELECT '' AS five, * FROM FLOAT4_TBL;
       | -1.23457e-20
 (5 rows)
 
+-- test edge-case coercions to integer
+SELECT '32767.4'::float4::int2;
+ int2  
+-------
+ 32767
+(1 row)
+
+SELECT '32767.6'::float4::int2;
+ERROR:  smallint out of range
+SELECT '-32768.4'::float4::int2;
+  int2  
+--------
+ -32768
+(1 row)
+
+SELECT '-32768.6'::float4::int2;
+ERROR:  smallint out of range
+SELECT '2147483520'::float4::int4;
+    int4    
+------------
+ 2147483520
+(1 row)
+
+SELECT '2147483647'::float4::int4;
+ERROR:  integer out of range
+SELECT '-2147483648.5'::float4::int4;
+    int4     
+-------------
+ -2147483648
+(1 row)
+
+SELECT '-2147483900'::float4::int4;
+ERROR:  integer out of range
+SELECT '9223369837831520256'::float4::int8;
+        int8         
+---------------------
+ 9223369837831520256
+(1 row)
+
+SELECT '9223372036854775807'::float4::int8;
+ERROR:  bigint out of range
+SELECT '-9223372036854775808.5'::float4::int8;
+         int8         
+----------------------
+ -9223372036854775808
+(1 row)
+
+SELECT '-9223380000000000000'::float4::int8;
+ERROR:  bigint out of range
index 5da743374c9174112fd7bfcaff2b17f25bd1d3a3..5f02dfb095ad47166006f045bcbe3322143b8d88 100644 (file)
@@ -442,3 +442,52 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
       | -1.2345678901234e-200
 (5 rows)
 
+-- test edge-case coercions to integer
+SELECT '32767.4'::float8::int2;
+ int2  
+-------
+ 32767
+(1 row)
+
+SELECT '32767.6'::float8::int2;
+ERROR:  smallint out of range
+SELECT '-32768.4'::float8::int2;
+  int2  
+--------
+ -32768
+(1 row)
+
+SELECT '-32768.6'::float8::int2;
+ERROR:  smallint out of range
+SELECT '2147483647.4'::float8::int4;
+    int4    
+------------
+ 2147483647
+(1 row)
+
+SELECT '2147483647.6'::float8::int4;
+ERROR:  integer out of range
+SELECT '-2147483648.4'::float8::int4;
+    int4     
+-------------
+ -2147483648
+(1 row)
+
+SELECT '-2147483648.6'::float8::int4;
+ERROR:  integer out of range
+SELECT '9223372036854773760'::float8::int8;
+        int8         
+---------------------
+ 9223372036854773760
+(1 row)
+
+SELECT '9223372036854775807'::float8::int8;
+ERROR:  bigint out of range
+SELECT '-9223372036854775808.5'::float8::int8;
+         int8         
+----------------------
+ -9223372036854775808
+(1 row)
+
+SELECT '-9223372036854780000'::float8::int8;
+ERROR:  bigint out of range
index 6221538af5c9b8f8adcbd3755eda467e94cc254d..bdd8eb45bddd8bed9d5a16929a9ca699d6012e8e 100644 (file)
@@ -444,3 +444,52 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
       | -1.2345678901234e-200
 (5 rows)
 
+-- test edge-case coercions to integer
+SELECT '32767.4'::float8::int2;
+ int2  
+-------
+ 32767
+(1 row)
+
+SELECT '32767.6'::float8::int2;
+ERROR:  smallint out of range
+SELECT '-32768.4'::float8::int2;
+  int2  
+--------
+ -32768
+(1 row)
+
+SELECT '-32768.6'::float8::int2;
+ERROR:  smallint out of range
+SELECT '2147483647.4'::float8::int4;
+    int4    
+------------
+ 2147483647
+(1 row)
+
+SELECT '2147483647.6'::float8::int4;
+ERROR:  integer out of range
+SELECT '-2147483648.4'::float8::int4;
+    int4     
+-------------
+ -2147483648
+(1 row)
+
+SELECT '-2147483648.6'::float8::int4;
+ERROR:  integer out of range
+SELECT '9223372036854773760'::float8::int8;
+        int8         
+---------------------
+ 9223372036854773760
+(1 row)
+
+SELECT '9223372036854775807'::float8::int8;
+ERROR:  bigint out of range
+SELECT '-9223372036854775808.5'::float8::int8;
+         int8         
+----------------------
+ -9223372036854775808
+(1 row)
+
+SELECT '-9223372036854780000'::float8::int8;
+ERROR:  bigint out of range
index 3b363f94635cfc29bcee3c65fe7a6e3031bfc205..46a9166d1319aba0e3a6c613f9e6d297921e5082 100644 (file)
@@ -81,3 +81,17 @@ UPDATE FLOAT4_TBL
    WHERE FLOAT4_TBL.f1 > '0.0';
 
 SELECT '' AS five, * FROM FLOAT4_TBL;
+
+-- test edge-case coercions to integer
+SELECT '32767.4'::float4::int2;
+SELECT '32767.6'::float4::int2;
+SELECT '-32768.4'::float4::int2;
+SELECT '-32768.6'::float4::int2;
+SELECT '2147483520'::float4::int4;
+SELECT '2147483647'::float4::int4;
+SELECT '-2147483648.5'::float4::int4;
+SELECT '-2147483900'::float4::int4;
+SELECT '9223369837831520256'::float4::int8;
+SELECT '9223372036854775807'::float4::int8;
+SELECT '-9223372036854775808.5'::float4::int8;
+SELECT '-9223380000000000000'::float4::int8;
index 92a574ab7bff24aa688d265cee98b4631cc19bc5..1b9d7a5b95730ee2cb2fb416badf87a5fb83e6cc 100644 (file)
@@ -167,3 +167,17 @@ INSERT INTO FLOAT8_TBL(f1) VALUES ('-1.2345678901234e+200');
 INSERT INTO FLOAT8_TBL(f1) VALUES ('-1.2345678901234e-200');
 
 SELECT '' AS five, * FROM FLOAT8_TBL;
+
+-- test edge-case coercions to integer
+SELECT '32767.4'::float8::int2;
+SELECT '32767.6'::float8::int2;
+SELECT '-32768.4'::float8::int2;
+SELECT '-32768.6'::float8::int2;
+SELECT '2147483647.4'::float8::int4;
+SELECT '2147483647.6'::float8::int4;
+SELECT '-2147483648.4'::float8::int4;
+SELECT '-2147483648.6'::float8::int4;
+SELECT '9223372036854773760'::float8::int8;
+SELECT '9223372036854775807'::float8::int8;
+SELECT '-9223372036854775808.5'::float8::int8;
+SELECT '-9223372036854780000'::float8::int8;