Fix float-to-integer coercions to handle edge cases correctly.

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.

Per bug #15519 from Victor Petrovykh.  This should get back-patched,
but first let's see what the buildfarm thinks of it --- I'm not too
sure about portability of some of the regression test cases.

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

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

diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index c91bb1a30597f919e34a35dfd105c99050799afa..cf9327f885325aa27f356b899a8d17c63e0d49ab 100644 (file)
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -1112,16 +1112,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 (unlikely(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);
 }
 
 
@@ -1133,12 +1145,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 (unlikely(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);
 }
 
 
@@ -1174,12 +1201,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 (unlikely(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);
 }
 
 
@@ -1191,12 +1233,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 (unlikely(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);
 }
 
 

diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 3c595e800a44d21c566ee83d7910e4375caaaf34..437d41816a2b6e8b86c43a3f496d1f025567c404 100644 (file)
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -1204,22 +1204,29 @@ i8tod(PG_FUNCTION_ARGS)
 Datum
 dtoi8(PG_FUNCTION_ARGS)
 {
-       float8          arg = PG_GETARG_FLOAT8(0);
-       int64           result;
+       float8          num = PG_GETARG_FLOAT8(0);
 
-       /* Round arg to nearest integer (but it's still in float form) */
-       arg = rint(arg);
+       /*
+        * 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);
 
-       if (unlikely(arg < (double) PG_INT64_MIN) ||
-               unlikely(arg > (double) PG_INT64_MAX) ||
-               unlikely(isnan(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 (unlikely(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")));
 
-       result = (int64) arg;
-
-       PG_RETURN_INT64(result);
+       PG_RETURN_INT64((int64) num);
 }
 
 Datum
@@ -1239,20 +1246,29 @@ i8tof(PG_FUNCTION_ARGS)
 Datum
 ftoi8(PG_FUNCTION_ARGS)
 {
-       float4          arg = PG_GETARG_FLOAT4(0);
-       float8          darg;
+       float4          num = PG_GETARG_FLOAT4(0);
 
-       /* Round arg to nearest integer (but it's still in float form) */
-       darg = rint(arg);
+       /*
+        * 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);
 
-       if (unlikely(arg < (float4) PG_INT64_MIN) ||
-               unlikely(arg > (float4) PG_INT64_MAX) ||
-               unlikely(isnan(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 (unlikely(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((int64) darg);
+       PG_RETURN_INT64((int64) num);
 }
 
 Datum

diff --git a/src/test/regress/expected/float4.out b/src/test/regress/expected/float4.out
index fd46a4a1db70da35bac51236c0b76d5f5514b189..2f47e1c202a9d6f3369d225434240f6bb501fab8 100644 (file)
--- a/src/test/regress/expected/float4.out
+++ b/src/test/regress/expected/float4.out
@@ -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

diff --git a/src/test/regress/expected/float8-small-is-zero.out b/src/test/regress/expected/float8-small-is-zero.out
index f8e09390f518c054569594db469e110788de2d6d..ce173f72c982b60a89a7674a47cc43aa74b94a2c 100644 (file)
--- a/src/test/regress/expected/float8-small-is-zero.out
+++ b/src/test/regress/expected/float8-small-is-zero.out
@@ -478,6 +478,55 @@ 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 '9223372036854774784'::float8::int8;
+        int8         
+---------------------
+ 9223372036854774784
+(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
 -- test exact cases for trigonometric functions in degrees
 SET extra_float_digits = 3;
 SELECT x,

diff --git a/src/test/regress/expected/float8.out b/src/test/regress/expected/float8.out
index b05831d45c954918bc65f0f168fb6f711a6fd4d9..1d51012a697136aae16008b943fa4c08b3f8335b 100644 (file)
--- a/src/test/regress/expected/float8.out
+++ b/src/test/regress/expected/float8.out
@@ -480,6 +480,55 @@ 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 '9223372036854774784'::float8::int8;
+        int8         
+---------------------
+ 9223372036854774784
+(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
 -- test exact cases for trigonometric functions in degrees
 SET extra_float_digits = 3;
 SELECT x,

diff --git a/src/test/regress/sql/float4.sql b/src/test/regress/sql/float4.sql
index 3b363f94635cfc29bcee3c65fe7a6e3031bfc205..46a9166d1319aba0e3a6c613f9e6d297921e5082 100644 (file)
--- a/src/test/regress/sql/float4.sql
+++ b/src/test/regress/sql/float4.sql
@@ -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;

diff --git a/src/test/regress/sql/float8.sql b/src/test/regress/sql/float8.sql
index eeebddd4b78d334ee73a777403b55060475c101a..ed8794244667ccfd40251202946c96d0e9d0cad5 100644 (file)
--- a/src/test/regress/sql/float8.sql
+++ b/src/test/regress/sql/float8.sql
@@ -174,6 +174,20 @@ 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 '9223372036854774784'::float8::int8;
+SELECT '9223372036854775807'::float8::int8;
+SELECT '-9223372036854775808.5'::float8::int8;
+SELECT '-9223372036854780000'::float8::int8;
+
 -- test exact cases for trigonometric functions in degrees
 SET extra_float_digits = 3;