Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b0e9e4d

Browse filesBrowse files
tglsfdctglsfdc
committed
Avoid overflow in width_bucket_float8().
The original coding of this function paid little attention to the possibility of overflow. There were actually three different hazards: 1. The range from bound1 to bound2 could exceed DBL_MAX, which on IEEE-compliant machines produces +Infinity in the subtraction. At best we'd lose all precision in the result, and at worst produce NaN due to dividing Inf/Inf. The range can't exceed twice DBL_MAX though, so we can fix this case by scaling all the inputs by 0.5. 2. We computed count * (operand - bound1), which is also at risk of float overflow, before dividing. Safer is to do the division first, producing a quotient that should be in [0,1), and even after allowing for roundoff error can't be outside [0,1]; then multiplying by count can't produce a result overflowing an int. (width_bucket_numeric does the multiplication first on the grounds that that improves accuracy of its result, but I don't think that a similar argument can be made in float arithmetic.) 3. If the division result does round to 1, and count is INT_MAX, the final addition of 1 would overflow an int. We took care of that in the operand >= bound2 case but did not consider that it could be possible in the main path. Fix that by moving the overflow-aware addition of 1 so it is done that way in all cases. The fix for point 2 creates a possibility that values very close to a bucket boundary will be rounded differently than they were before. I'm not troubled by that for HEAD, but it is an argument against putting this into the stable branches. Given that the cases being fixed here are fairly extreme and unlikely to be hit in normal use, it seems best not to back-patch. Mats Kindahl and Tom Lane Discussion: https://postgr.es/m/17876-61f280d1601f978d@postgresql.org
1 parent 2fe7a6d commit b0e9e4d
Copy full SHA for b0e9e4d

File tree

3 files changed

+89
-15
lines changed
Filter options

3 files changed

+89
-15
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/float.c
+28-15Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,7 +4090,7 @@ width_bucket_float8(PG_FUNCTION_ARGS)
40904090
int32 count = PG_GETARG_INT32(3);
40914091
int32 result;
40924092

4093-
if (count <= 0.0)
4093+
if (count <= 0)
40944094
ereport(ERROR,
40954095
(errcode(ERRCODE_INVALID_ARGUMENT_FOR_WIDTH_BUCKET_FUNCTION),
40964096
errmsg("count must be greater than zero")));
@@ -4108,31 +4108,39 @@ width_bucket_float8(PG_FUNCTION_ARGS)
41084108

41094109
if (bound1 < bound2)
41104110
{
4111+
/* In all cases, we'll add one at the end */
41114112
if (operand < bound1)
4112-
result = 0;
4113+
result = -1;
41134114
else if (operand >= bound2)
4115+
result = count;
4116+
else if (!isinf(bound2 - bound1))
41144117
{
4115-
if (pg_add_s32_overflow(count, 1, &result))
4116-
ereport(ERROR,
4117-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
4118-
errmsg("integer out of range")));
4118+
/* Result of division is surely in [0,1], so this can't overflow */
4119+
result = count * ((operand - bound1) / (bound2 - bound1));
41194120
}
41204121
else
4121-
result = ((float8) count * (operand - bound1) / (bound2 - bound1)) + 1;
4122+
{
4123+
/*
4124+
* We get here if bound2 - bound1 overflows DBL_MAX. Since both
4125+
* bounds are finite, their difference can't exceed twice DBL_MAX;
4126+
* so we can perform the computation without overflow by dividing
4127+
* all the inputs by 2. That should be exact, too, except in the
4128+
* case where a very small operand underflows to zero, which would
4129+
* have negligible impact on the result given such large bounds.
4130+
*/
4131+
result = count * ((operand / 2 - bound1 / 2) / (bound2 / 2 - bound1 / 2));
4132+
}
41224133
}
41234134
else if (bound1 > bound2)
41244135
{
41254136
if (operand > bound1)
4126-
result = 0;
4137+
result = -1;
41274138
else if (operand <= bound2)
4128-
{
4129-
if (pg_add_s32_overflow(count, 1, &result))
4130-
ereport(ERROR,
4131-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
4132-
errmsg("integer out of range")));
4133-
}
4139+
result = count;
4140+
else if (!isinf(bound1 - bound2))
4141+
result = count * ((bound1 - operand) / (bound1 - bound2));
41344142
else
4135-
result = ((float8) count * (bound1 - operand) / (bound1 - bound2)) + 1;
4143+
result = count * ((bound1 / 2 - operand / 2) / (bound1 / 2 - bound2 / 2));
41364144
}
41374145
else
41384146
{
@@ -4142,5 +4150,10 @@ width_bucket_float8(PG_FUNCTION_ARGS)
41424150
result = 0; /* keep the compiler quiet */
41434151
}
41444152

4153+
if (pg_add_s32_overflow(result, 1, &result))
4154+
ereport(ERROR,
4155+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
4156+
errmsg("integer out of range")));
4157+
41454158
PG_RETURN_INT32(result);
41464159
}

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

Copy file name to clipboardExpand all lines: src/test/regress/expected/numeric.out
+39Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,45 @@ FROM generate_series(0, 110, 10) x;
14731473
110 | 0 | 0
14741474
(12 rows)
14751475

1476+
-- Check cases that could trigger overflow or underflow within the calculation
1477+
SELECT oper, low, high, cnt, width_bucket(oper, low, high, cnt)
1478+
FROM
1479+
(SELECT 1.797e+308::float8 AS big, 5e-324::float8 AS tiny) as v,
1480+
LATERAL (VALUES
1481+
(10.5::float8, -big, big, 1),
1482+
(10.5::float8, -big, big, 2),
1483+
(10.5::float8, -big, big, 3),
1484+
(big / 4, -big / 2, big / 2, 10),
1485+
(10.5::float8, big, -big, 1),
1486+
(10.5::float8, big, -big, 2),
1487+
(10.5::float8, big, -big, 3),
1488+
(big / 4, big / 2, -big / 2, 10),
1489+
(0, 0, tiny, 4),
1490+
(tiny, 0, tiny, 4),
1491+
(0, 0, 1, 2147483647),
1492+
(1, 1, 0, 2147483647)
1493+
) as sample(oper, low, high, cnt);
1494+
oper | low | high | cnt | width_bucket
1495+
-------------+-------------+-------------+------------+--------------
1496+
10.5 | -1.797e+308 | 1.797e+308 | 1 | 1
1497+
10.5 | -1.797e+308 | 1.797e+308 | 2 | 2
1498+
10.5 | -1.797e+308 | 1.797e+308 | 3 | 2
1499+
4.4925e+307 | -8.985e+307 | 8.985e+307 | 10 | 8
1500+
10.5 | 1.797e+308 | -1.797e+308 | 1 | 1
1501+
10.5 | 1.797e+308 | -1.797e+308 | 2 | 2
1502+
10.5 | 1.797e+308 | -1.797e+308 | 3 | 2
1503+
4.4925e+307 | 8.985e+307 | -8.985e+307 | 10 | 3
1504+
0 | 0 | 5e-324 | 4 | 1
1505+
5e-324 | 0 | 5e-324 | 4 | 5
1506+
0 | 0 | 1 | 2147483647 | 1
1507+
1 | 1 | 0 | 2147483647 | 1
1508+
(12 rows)
1509+
1510+
-- These fail because the result would be out of int32 range:
1511+
SELECT width_bucket(1::float8, 0, 1, 2147483647);
1512+
ERROR: integer out of range
1513+
SELECT width_bucket(0::float8, 1, 0, 2147483647);
1514+
ERROR: integer out of range
14761515
--
14771516
-- TO_CHAR()
14781517
--

‎src/test/regress/sql/numeric.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/numeric.sql
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,28 @@ SELECT x, width_bucket(x::float8, 100, 10, 9) as flt,
910910
width_bucket(x::numeric, 100, 10, 9) as num
911911
FROM generate_series(0, 110, 10) x;
912912

913+
-- Check cases that could trigger overflow or underflow within the calculation
914+
SELECT oper, low, high, cnt, width_bucket(oper, low, high, cnt)
915+
FROM
916+
(SELECT 1.797e+308::float8 AS big, 5e-324::float8 AS tiny) as v,
917+
LATERAL (VALUES
918+
(10.5::float8, -big, big, 1),
919+
(10.5::float8, -big, big, 2),
920+
(10.5::float8, -big, big, 3),
921+
(big / 4, -big / 2, big / 2, 10),
922+
(10.5::float8, big, -big, 1),
923+
(10.5::float8, big, -big, 2),
924+
(10.5::float8, big, -big, 3),
925+
(big / 4, big / 2, -big / 2, 10),
926+
(0, 0, tiny, 4),
927+
(tiny, 0, tiny, 4),
928+
(0, 0, 1, 2147483647),
929+
(1, 1, 0, 2147483647)
930+
) as sample(oper, low, high, cnt);
931+
-- These fail because the result would be out of int32 range:
932+
SELECT width_bucket(1::float8, 0, 1, 2147483647);
933+
SELECT width_bucket(0::float8, 1, 0, 2147483647);
934+
913935
--
914936
-- TO_CHAR()
915937
--

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.