Skip to content

Commit d523125

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. This back-patches commits cbdb8b4 and 452b637. In the 9.4 branch, also back-patch the portion of 62e2a8d 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
1 parent 2f30b31 commit d523125

File tree

8 files changed

+290
-34
lines changed

8 files changed

+290
-34
lines changed

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

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,16 +1149,28 @@ Datum
11491149
dtoi4(PG_FUNCTION_ARGS)
11501150
{
11511151
float8 num = PG_GETARG_FLOAT8(0);
1152-
int32 result;
11531152

1154-
/* 'Inf' is handled by INT_MAX */
1155-
if (num < INT_MIN || num > INT_MAX || isnan(num))
1153+
/*
1154+
* Get rid of any fractional part in the input. This is so we don't fail
1155+
* on just-out-of-range values that would round into range. Note
1156+
* assumption that rint() will pass through a NaN or Inf unchanged.
1157+
*/
1158+
num = rint(num);
1159+
1160+
/*
1161+
* Range check. We must be careful here that the boundary values are
1162+
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1163+
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1164+
* isn't, and might get rounded off, so avoid using it.
1165+
*/
1166+
if (num < (float8) PG_INT32_MIN ||
1167+
num >= -((float8) PG_INT32_MIN) ||
1168+
isnan(num))
11561169
ereport(ERROR,
11571170
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11581171
errmsg("integer out of range")));
11591172

1160-
result = (int32) rint(num);
1161-
PG_RETURN_INT32(result);
1173+
PG_RETURN_INT32((int32) num);
11621174
}
11631175

11641176

@@ -1170,12 +1182,27 @@ dtoi2(PG_FUNCTION_ARGS)
11701182
{
11711183
float8 num = PG_GETARG_FLOAT8(0);
11721184

1173-
if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
1185+
/*
1186+
* Get rid of any fractional part in the input. This is so we don't fail
1187+
* on just-out-of-range values that would round into range. Note
1188+
* assumption that rint() will pass through a NaN or Inf unchanged.
1189+
*/
1190+
num = rint(num);
1191+
1192+
/*
1193+
* Range check. We must be careful here that the boundary values are
1194+
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1195+
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1196+
* isn't, and might get rounded off, so avoid using it.
1197+
*/
1198+
if (num < (float8) PG_INT16_MIN ||
1199+
num >= -((float8) PG_INT16_MIN) ||
1200+
isnan(num))
11741201
ereport(ERROR,
11751202
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
11761203
errmsg("smallint out of range")));
11771204

1178-
PG_RETURN_INT16((int16) rint(num));
1205+
PG_RETURN_INT16((int16) num);
11791206
}
11801207

11811208

@@ -1211,12 +1238,27 @@ ftoi4(PG_FUNCTION_ARGS)
12111238
{
12121239
float4 num = PG_GETARG_FLOAT4(0);
12131240

1214-
if (num < INT_MIN || num > INT_MAX || isnan(num))
1241+
/*
1242+
* Get rid of any fractional part in the input. This is so we don't fail
1243+
* on just-out-of-range values that would round into range. Note
1244+
* assumption that rint() will pass through a NaN or Inf unchanged.
1245+
*/
1246+
num = rint(num);
1247+
1248+
/*
1249+
* Range check. We must be careful here that the boundary values are
1250+
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1251+
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1252+
* isn't, and might get rounded off, so avoid using it.
1253+
*/
1254+
if (num < (float4) PG_INT32_MIN ||
1255+
num >= -((float4) PG_INT32_MIN) ||
1256+
isnan(num))
12151257
ereport(ERROR,
12161258
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12171259
errmsg("integer out of range")));
12181260

1219-
PG_RETURN_INT32((int32) rint(num));
1261+
PG_RETURN_INT32((int32) num);
12201262
}
12211263

12221264

@@ -1228,12 +1270,27 @@ ftoi2(PG_FUNCTION_ARGS)
12281270
{
12291271
float4 num = PG_GETARG_FLOAT4(0);
12301272

1231-
if (num < SHRT_MIN || num > SHRT_MAX || isnan(num))
1273+
/*
1274+
* Get rid of any fractional part in the input. This is so we don't fail
1275+
* on just-out-of-range values that would round into range. Note
1276+
* assumption that rint() will pass through a NaN or Inf unchanged.
1277+
*/
1278+
num = rint(num);
1279+
1280+
/*
1281+
* Range check. We must be careful here that the boundary values are
1282+
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1283+
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1284+
* isn't, and might get rounded off, so avoid using it.
1285+
*/
1286+
if (num < (float4) PG_INT16_MIN ||
1287+
num >= -((float4) PG_INT16_MIN) ||
1288+
isnan(num))
12321289
ereport(ERROR,
12331290
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12341291
errmsg("smallint out of range")));
12351292

1236-
PG_RETURN_INT16((int16) rint(num));
1293+
PG_RETURN_INT16((int16) num);
12371294
}
12381295

12391296

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

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,25 +1342,29 @@ i8tod(PG_FUNCTION_ARGS)
13421342
Datum
13431343
dtoi8(PG_FUNCTION_ARGS)
13441344
{
1345-
float8 arg = PG_GETARG_FLOAT8(0);
1346-
int64 result;
1347-
1348-
/* Round arg to nearest integer (but it's still in float form) */
1349-
arg = rint(arg);
1345+
float8 num = PG_GETARG_FLOAT8(0);
13501346

13511347
/*
1352-
* Does it fit in an int64? Avoid assuming that we have handy constants
1353-
* defined for the range boundaries, instead test for overflow by
1354-
* reverse-conversion.
1348+
* Get rid of any fractional part in the input. This is so we don't fail
1349+
* on just-out-of-range values that would round into range. Note
1350+
* assumption that rint() will pass through a NaN or Inf unchanged.
13551351
*/
1356-
result = (int64) arg;
1352+
num = rint(num);
13571353

1358-
if ((float8) result != arg)
1354+
/*
1355+
* Range check. We must be careful here that the boundary values are
1356+
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1357+
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1358+
* isn't, and might get rounded off, so avoid using it.
1359+
*/
1360+
if (num < (float8) PG_INT64_MIN ||
1361+
num >= -((float8) PG_INT64_MIN) ||
1362+
isnan(num))
13591363
ereport(ERROR,
13601364
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13611365
errmsg("bigint out of range")));
13621366

1363-
PG_RETURN_INT64(result);
1367+
PG_RETURN_INT64((int64) num);
13641368
}
13651369

13661370
Datum
@@ -1380,26 +1384,29 @@ i8tof(PG_FUNCTION_ARGS)
13801384
Datum
13811385
ftoi8(PG_FUNCTION_ARGS)
13821386
{
1383-
float4 arg = PG_GETARG_FLOAT4(0);
1384-
int64 result;
1385-
float8 darg;
1386-
1387-
/* Round arg to nearest integer (but it's still in float form) */
1388-
darg = rint(arg);
1387+
float4 num = PG_GETARG_FLOAT4(0);
13891388

13901389
/*
1391-
* Does it fit in an int64? Avoid assuming that we have handy constants
1392-
* defined for the range boundaries, instead test for overflow by
1393-
* reverse-conversion.
1390+
* Get rid of any fractional part in the input. This is so we don't fail
1391+
* on just-out-of-range values that would round into range. Note
1392+
* assumption that rint() will pass through a NaN or Inf unchanged.
13941393
*/
1395-
result = (int64) darg;
1394+
num = rint(num);
13961395

1397-
if ((float8) result != darg)
1396+
/*
1397+
* Range check. We must be careful here that the boundary values are
1398+
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1399+
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1400+
* isn't, and might get rounded off, so avoid using it.
1401+
*/
1402+
if (num < (float4) PG_INT64_MIN ||
1403+
num >= -((float4) PG_INT64_MIN) ||
1404+
isnan(num))
13981405
ereport(ERROR,
13991406
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
14001407
errmsg("bigint out of range")));
14011408

1402-
PG_RETURN_INT64(result);
1409+
PG_RETURN_INT64((int64) num);
14031410
}
14041411

14051412
Datum

‎src/include/c.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,23 @@ typedef unsigned long long int uint64;
283283
#error must have a working 64-bit integer datatype
284284
#endif
285285

286+
/*
287+
* stdint.h limits aren't guaranteed to be present and aren't guaranteed to
288+
* have compatible types with our fixed width types. So just define our own.
289+
*/
290+
#define PG_INT8_MIN (-0x7F-1)
291+
#define PG_INT8_MAX (0x7F)
292+
#define PG_UINT8_MAX (0xFF)
293+
#define PG_INT16_MIN (-0x7FFF-1)
294+
#define PG_INT16_MAX (0x7FFF)
295+
#define PG_UINT16_MAX (0xFFFF)
296+
#define PG_INT32_MIN (-0x7FFFFFFF-1)
297+
#define PG_INT32_MAX (0x7FFFFFFF)
298+
#define PG_UINT32_MAX (0xFFFFFFFFU)
299+
#define PG_INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
300+
#define PG_INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
301+
#define PG_UINT64_MAX UINT64CONST(0xFFFFFFFFFFFFFFFF)
302+
286303
/* Max value of size_t might be missing if we don't have stdint.h */
287304
#ifndef SIZE_MAX
288305
#if SIZEOF_SIZE_T == 8

‎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
@@ -442,3 +442,52 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
442442
| -1.2345678901234e-200
443443
(5 rows)
444444

445+
-- test edge-case coercions to integer
446+
SELECT '32767.4'::float8::int2;
447+
int2
448+
-------
449+
32767
450+
(1 row)
451+
452+
SELECT '32767.6'::float8::int2;
453+
ERROR: smallint out of range
454+
SELECT '-32768.4'::float8::int2;
455+
int2
456+
--------
457+
-32768
458+
(1 row)
459+
460+
SELECT '-32768.6'::float8::int2;
461+
ERROR: smallint out of range
462+
SELECT '2147483647.4'::float8::int4;
463+
int4
464+
------------
465+
2147483647
466+
(1 row)
467+
468+
SELECT '2147483647.6'::float8::int4;
469+
ERROR: integer out of range
470+
SELECT '-2147483648.4'::float8::int4;
471+
int4
472+
-------------
473+
-2147483648
474+
(1 row)
475+
476+
SELECT '-2147483648.6'::float8::int4;
477+
ERROR: integer out of range
478+
SELECT '9223372036854773760'::float8::int8;
479+
int8
480+
---------------------
481+
9223372036854773760
482+
(1 row)
483+
484+
SELECT '9223372036854775807'::float8::int8;
485+
ERROR: bigint out of range
486+
SELECT '-9223372036854775808.5'::float8::int8;
487+
int8
488+
----------------------
489+
-9223372036854775808
490+
(1 row)
491+
492+
SELECT '-9223372036854780000'::float8::int8;
493+
ERROR: bigint out of range

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,3 +444,52 @@ SELECT '' AS five, * FROM FLOAT8_TBL;
444444
| -1.2345678901234e-200
445445
(5 rows)
446446

447+
-- test edge-case coercions to integer
448+
SELECT '32767.4'::float8::int2;
449+
int2
450+
-------
451+
32767
452+
(1 row)
453+
454+
SELECT '32767.6'::float8::int2;
455+
ERROR: smallint out of range
456+
SELECT '-32768.4'::float8::int2;
457+
int2
458+
--------
459+
-32768
460+
(1 row)
461+
462+
SELECT '-32768.6'::float8::int2;
463+
ERROR: smallint out of range
464+
SELECT '2147483647.4'::float8::int4;
465+
int4
466+
------------
467+
2147483647
468+
(1 row)
469+
470+
SELECT '2147483647.6'::float8::int4;
471+
ERROR: integer out of range
472+
SELECT '-2147483648.4'::float8::int4;
473+
int4
474+
-------------
475+
-2147483648
476+
(1 row)
477+
478+
SELECT '-2147483648.6'::float8::int4;
479+
ERROR: integer out of range
480+
SELECT '9223372036854773760'::float8::int8;
481+
int8
482+
---------------------
483+
9223372036854773760
484+
(1 row)
485+
486+
SELECT '9223372036854775807'::float8::int8;
487+
ERROR: bigint out of range
488+
SELECT '-9223372036854775808.5'::float8::int8;
489+
int8
490+
----------------------
491+
-9223372036854775808
492+
(1 row)
493+
494+
SELECT '-9223372036854780000'::float8::int8;
495+
ERROR: bigint out of range

0 commit comments

Comments
 (0)