Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

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

 Sign up
Appearance settings

Commitb0e9e4d

Browse files
committed
Avoid overflow in width_bucket_float8().
The original coding of this function paid little attention to thepossibility of overflow. There were actually three different hazards:1. The range from bound1 to bound2 could exceed DBL_MAX, which onIEEE-compliant machines produces +Infinity in the subtraction.At best we'd lose all precision in the result, and at worstproduce NaN due to dividing Inf/Inf. The range can't exceedtwice DBL_MAX though, so we can fix this case by scaling all theinputs by 0.5.2. We computed count * (operand - bound1), which is also at risk offloat overflow, before dividing. Safer is to do the division first,producing a quotient that should be in [0,1), and even after allowingfor roundoff error can't be outside [0,1]; then multiplying by countcan't produce a result overflowing an int. (width_bucket_numeric doesthe multiplication first on the grounds that that improves accuracy ofits result, but I don't think that a similar argument can be made infloat 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 careof that in the operand >= bound2 case but did not consider thatit could be possible in the main path. Fix that by moving theoverflow-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 toa bucket boundary will be rounded differently than they were before.I'm not troubled by that for HEAD, but it is an argument againstputting this into the stable branches. Given that the cases beingfixed here are fairly extreme and unlikely to be hit in normal use,it seems best not to back-patch.Mats Kindahl and Tom LaneDiscussion:https://postgr.es/m/17876-61f280d1601f978d@postgresql.org
1 parent2fe7a6d commitb0e9e4d

File tree

3 files changed

+89
-15
lines changed

3 files changed

+89
-15
lines changed

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

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,7 +4090,7 @@ width_bucket_float8(PG_FUNCTION_ARGS)
40904090
int32count=PG_GETARG_INT32(3);
40914091
int32result;
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
elseif (operand >=bound2)
4115+
result=count;
4116+
elseif (!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
elseif (bound1>bound2)
41244135
{
41254136
if (operand>bound1)
4126-
result=0;
4137+
result=-1;
41274138
elseif (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+
elseif (!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

Lines 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

Lines 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+
(SELECT1.797e+308::float8AS big, 5e-324::float8AS 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)
[8]ページ先頭
©2009-2025 Movatter.jp