Skip to content

Commit cbdb8b4

Browse files
tglsfdctglsfdc
committed
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
1 parent eb6f291 commit cbdb8b4

File tree

7 files changed

+277
-29
lines changed

7 files changed

+277
-29
lines changed

‎src/backend/utils/adt/float.c

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,16 +1112,28 @@ Datum
11121112
dtoi4(PG_FUNCTION_ARGS)
11131113
{
11141114
float8 num = PG_GETARG_FLOAT8(0);
1115-
int32 result;
11161115

1117-
/* 'Inf' is handled by INT_MAX */
1118-
if (num < INT_MIN || num > INT_MAX || isnan(num))
1116+
/*
1117+
* Get rid of any fractional part in the input. This is so we don't fail
1118+
* on just-out-of-range values that would round into range. Note
1119+
* assumption that rint() will pass through a NaN or Inf unchanged.
1120+
*/
1121+
num = rint(num);
1122+
1123+
/*
1124+
* Range check. We must be careful here that the boundary values are
1125+
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1126+
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1127+
* isn't, and might get rounded off, so avoid using it.
1128+
*/
1129+
if (unlikely(num < (float8) PG_INT32_MIN ||
1130+
num >= -((float8) PG_INT32_MIN) ||
1131+
isnan(num)))
11191132
ereport(ERROR,
11201133
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11211134
errmsg("integer out of range")));
11221135

1123-
result = (int32) rint(num);
1124-
PG_RETURN_INT32(result);
1136+
PG_RETURN_INT32((int32) num);
11251137
}
11261138

11271139

@@ -1133,12 +1145,27 @@ dtoi2(PG_FUNCTION_ARGS)
11331145
{
11341146
float8 num = PG_GETARG_FLOAT8(0);
11351147

1136-
if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
1148+
/*
1149+
* Get rid of any fractional part in the input. This is so we don't fail
1150+
* on just-out-of-range values that would round into range. Note
1151+
* assumption that rint() will pass through a NaN or Inf unchanged.
1152+
*/
1153+
num = rint(num);
1154+
1155+
/*
1156+
* Range check. We must be careful here that the boundary values are
1157+
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1158+
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1159+
* isn't, and might get rounded off, so avoid using it.
1160+
*/
1161+
if (unlikely(num < (float8) PG_INT16_MIN ||
1162+
num >= -((float8) PG_INT16_MIN) ||
1163+
isnan(num)))
11371164
ereport(ERROR,
11381165
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11391166
errmsg("smallint out of range")));
11401167

1141-
PG_RETURN_INT16((int16) rint(num));
1168+
PG_RETURN_INT16((int16) num);
11421169
}
11431170

11441171

@@ -1174,12 +1201,27 @@ ftoi4(PG_FUNCTION_ARGS)
11741201
{
11751202
float4 num = PG_GETARG_FLOAT4(0);
11761203

1177-
if (num < INT_MIN || num > INT_MAX || isnan(num))
1204+
/*
1205+
* Get rid of any fractional part in the input. This is so we don't fail
1206+
* on just-out-of-range values that would round into range. Note
1207+
* assumption that rint() will pass through a NaN or Inf unchanged.
1208+
*/
1209+
num = rint(num);
1210+
1211+
/*
1212+
* Range check. We must be careful here that the boundary values are
1213+
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1214+
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1215+
* isn't, and might get rounded off, so avoid using it.
1216+
*/
1217+
if (unlikely(num < (float4) PG_INT32_MIN ||
1218+
num >= -((float4) PG_INT32_MIN) ||
1219+
isnan(num)))
11781220
ereport(ERROR,
11791221
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11801222
errmsg("integer out of range")));
11811223

1182-
PG_RETURN_INT32((int32) rint(num));
1224+
PG_RETURN_INT32((int32) num);
11831225
}
11841226

11851227

@@ -1191,12 +1233,27 @@ ftoi2(PG_FUNCTION_ARGS)
11911233
{
11921234
float4 num = PG_GETARG_FLOAT4(0);
11931235

1194-
if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
1236+
/*
1237+
* Get rid of any fractional part in the input. This is so we don't fail
1238+
* on just-out-of-range values that would round into range. Note
1239+
* assumption that rint() will pass through a NaN or Inf unchanged.
1240+
*/
1241+
num = rint(num);
1242+
1243+
/*
1244+
* Range check. We must be careful here that the boundary values are
1245+
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1246+
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1247+
* isn't, and might get rounded off, so avoid using it.
1248+
*/
1249+
if (unlikely(num < (float4) PG_INT16_MIN ||
1250+
num >= -((float4) PG_INT16_MIN) ||
1251+
isnan(num)))
11951252
ereport(ERROR,
11961253
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11971254
errmsg("smallint out of range")));
11981255

1199-
PG_RETURN_INT16((int16) rint(num));
1256+
PG_RETURN_INT16((int16) num);
12001257
}
12011258

12021259

‎src/backend/utils/adt/int8.c

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,22 +1204,29 @@ i8tod(PG_FUNCTION_ARGS)
12041204
Datum
12051205
dtoi8(PG_FUNCTION_ARGS)
12061206
{
1207-
float8 arg = PG_GETARG_FLOAT8(0);
1208-
int64 result;
1207+
float8 num = PG_GETARG_FLOAT8(0);
12091208

1210-
/* Round arg to nearest integer (but it's still in float form) */
1211-
arg = rint(arg);
1209+
/*
1210+
* Get rid of any fractional part in the input. This is so we don't fail
1211+
* on just-out-of-range values that would round into range. Note
1212+
* assumption that rint() will pass through a NaN or Inf unchanged.
1213+
*/
1214+
num = rint(num);
12121215

1213-
if (unlikely(arg < (double) PG_INT64_MIN) ||
1214-
unlikely(arg > (double) PG_INT64_MAX) ||
1215-
unlikely(isnan(arg)))
1216+
/*
1217+
* Range check. We must be careful here that the boundary values are
1218+
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1219+
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1220+
* isn't, and might get rounded off, so avoid using it.
1221+
*/
1222+
if (unlikely(num < (float8) PG_INT64_MIN ||
1223+
num >= -((float8) PG_INT64_MIN) ||
1224+
isnan(num)))
12161225
ereport(ERROR,
12171226
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12181227
errmsg("bigint out of range")));
12191228

1220-
result = (int64) arg;
1221-
1222-
PG_RETURN_INT64(result);
1229+
PG_RETURN_INT64((int64) num);
12231230
}
12241231

12251232
Datum
@@ -1239,20 +1246,29 @@ i8tof(PG_FUNCTION_ARGS)
12391246
Datum
12401247
ftoi8(PG_FUNCTION_ARGS)
12411248
{
1242-
float4 arg = PG_GETARG_FLOAT4(0);
1243-
float8 darg;
1249+
float4 num = PG_GETARG_FLOAT4(0);
12441250

1245-
/* Round arg to nearest integer (but it's still in float form) */
1246-
darg = rint(arg);
1251+
/*
1252+
* Get rid of any fractional part in the input. This is so we don't fail
1253+
* on just-out-of-range values that would round into range. Note
1254+
* assumption that rint() will pass through a NaN or Inf unchanged.
1255+
*/
1256+
num = rint(num);
12471257

1248-
if (unlikely(arg < (float4) PG_INT64_MIN) ||
1249-
unlikely(arg > (float4) PG_INT64_MAX) ||
1250-
unlikely(isnan(arg)))
1258+
/*
1259+
* Range check. We must be careful here that the boundary values are
1260+
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1261+
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1262+
* isn't, and might get rounded off, so avoid using it.
1263+
*/
1264+
if (unlikely(num < (float4) PG_INT64_MIN ||
1265+
num >= -((float4) PG_INT64_MIN) ||
1266+
isnan(num)))
12511267
ereport(ERROR,
12521268
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12531269
errmsg("bigint out of range")));
12541270

1255-
PG_RETURN_INT64((int64) darg);
1271+
PG_RETURN_INT64((int64) num);
12561272
}
12571273

12581274
Datum

‎src/test/regress/expected/float4.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,52 @@ SELECT '' AS five, * FROM FLOAT4_TBL;
257257
| -1.23457e-20
258258
(5 rows)
259259

260+
-- test edge-case coercions to integer
261+
SELECT '32767.4'::float4::int2;
262+
int2
263+
-------
264+
32767
265+
(1 row)
266+
267+
SELECT '32767.6'::float4::int2;
268+
ERROR: smallint out of range
269+
SELECT '-32768.4'::float4::int2;
270+
int2
271+
--------
272+
-32768
273+
(1 row)
274+
275+
SELECT '-32768.6'::float4::int2;
276+
ERROR: smallint out of range
277+
SELECT '2147483520'::float4::int4;
278+
int4
279+
------------
280+
2147483520
281+
(1 row)
282+
283+
SELECT '2147483647'::float4::int4;
284+
ERROR: integer out of range
285+
SELECT '-2147483648.5'::float4::int4;
286+
int4
287+
-------------
288+
-2147483648
289+
(1 row)
290+
291+
SELECT '-2147483900'::float4::int4;
292+
ERROR: integer out of range
293+
SELECT '9223369837831520256'::float4::int8;
294+
int8
295+
---------------------
296+
9223369837831520256
297+
(1 row)
298+
299+
SELECT '9223372036854775807'::float4::int8;
300+
ERROR: bigint out of range
301+
SELECT '-9223372036854775808.5'::float4::int8;
302+
int8
303+
----------------------
304+
-9223372036854775808
305+
(1 row)
306+
307+
SELECT '-9223380000000000000'::float4::int8;
308+
ERROR: bigint out of range

‎src/test/regress/expected/float8-small-is-zero.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,55 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
478478
| -1.2345678901234e-200
479479
(5 rows)
480480

481+
-- test edge-case coercions to integer
482+
SELECT '32767.4'::float8::int2;
483+
int2
484+
-------
485+
32767
486+
(1 row)
487+
488+
SELECT '32767.6'::float8::int2;
489+
ERROR: smallint out of range
490+
SELECT '-32768.4'::float8::int2;
491+
int2
492+
--------
493+
-32768
494+
(1 row)
495+
496+
SELECT '-32768.6'::float8::int2;
497+
ERROR: smallint out of range
498+
SELECT '2147483647.4'::float8::int4;
499+
int4
500+
------------
501+
2147483647
502+
(1 row)
503+
504+
SELECT '2147483647.6'::float8::int4;
505+
ERROR: integer out of range
506+
SELECT '-2147483648.4'::float8::int4;
507+
int4
508+
-------------
509+
-2147483648
510+
(1 row)
511+
512+
SELECT '-2147483648.6'::float8::int4;
513+
ERROR: integer out of range
514+
SELECT '9223372036854774784'::float8::int8;
515+
int8
516+
---------------------
517+
9223372036854774784
518+
(1 row)
519+
520+
SELECT '9223372036854775807'::float8::int8;
521+
ERROR: bigint out of range
522+
SELECT '-9223372036854775808.5'::float8::int8;
523+
int8
524+
----------------------
525+
-9223372036854775808
526+
(1 row)
527+
528+
SELECT '-9223372036854780000'::float8::int8;
529+
ERROR: bigint out of range
481530
-- test exact cases for trigonometric functions in degrees
482531
SET extra_float_digits = 3;
483532
SELECT x,

‎src/test/regress/expected/float8.out

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,55 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
480480
| -1.2345678901234e-200
481481
(5 rows)
482482

483+
-- test edge-case coercions to integer
484+
SELECT '32767.4'::float8::int2;
485+
int2
486+
-------
487+
32767
488+
(1 row)
489+
490+
SELECT '32767.6'::float8::int2;
491+
ERROR: smallint out of range
492+
SELECT '-32768.4'::float8::int2;
493+
int2
494+
--------
495+
-32768
496+
(1 row)
497+
498+
SELECT '-32768.6'::float8::int2;
499+
ERROR: smallint out of range
500+
SELECT '2147483647.4'::float8::int4;
501+
int4
502+
------------
503+
2147483647
504+
(1 row)
505+
506+
SELECT '2147483647.6'::float8::int4;
507+
ERROR: integer out of range
508+
SELECT '-2147483648.4'::float8::int4;
509+
int4
510+
-------------
511+
-2147483648
512+
(1 row)
513+
514+
SELECT '-2147483648.6'::float8::int4;
515+
ERROR: integer out of range
516+
SELECT '9223372036854774784'::float8::int8;
517+
int8
518+
---------------------
519+
9223372036854774784
520+
(1 row)
521+
522+
SELECT '9223372036854775807'::float8::int8;
523+
ERROR: bigint out of range
524+
SELECT '-9223372036854775808.5'::float8::int8;
525+
int8
526+
----------------------
527+
-9223372036854775808
528+
(1 row)
529+
530+
SELECT '-9223372036854780000'::float8::int8;
531+
ERROR: bigint out of range
483532
-- test exact cases for trigonometric functions in degrees
484533
SET extra_float_digits = 3;
485534
SELECT x,

‎src/test/regress/sql/float4.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,17 @@ UPDATE FLOAT4_TBL
8181
WHERE FLOAT4_TBL.f1 > '0.0';
8282

8383
SELECT '' AS five, * FROM FLOAT4_TBL;
84+
85+
-- test edge-case coercions to integer
86+
SELECT '32767.4'::float4::int2;
87+
SELECT '32767.6'::float4::int2;
88+
SELECT '-32768.4'::float4::int2;
89+
SELECT '-32768.6'::float4::int2;
90+
SELECT '2147483520'::float4::int4;
91+
SELECT '2147483647'::float4::int4;
92+
SELECT '-2147483648.5'::float4::int4;
93+
SELECT '-2147483900'::float4::int4;
94+
SELECT '9223369837831520256'::float4::int8;
95+
SELECT '9223372036854775807'::float4::int8;
96+
SELECT '-9223372036854775808.5'::float4::int8;
97+
SELECT '-9223380000000000000'::float4::int8;

0 commit comments

Comments
 (0)