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

Commita2a0c7c

Browse files
committed
Further tweaking of width_bucket() edge cases.
I realized that the third overflow case I posited in commitb0e9e4dactually should be handled in a different way: rather than toleratingthe idea that the quotient could round to 1, we should clamp so thatthe output cannot be more than "count" when we know that the operand isless than bound2. That being the case, we don't need an overflow-awareincrement in that code path, which leads me to revert the movement ofthe pg_add_s32_overflow() call. (The diff in width_bucket_float8might be easier to read by comparing againstb0e9e4d^.)What's more, width_bucket_numeric also has this problem of the quotientpotentially rounding to 1, so add a clamp there too.As before, I'm not quite convinced that a back-patch is warranted.Discussion:https://postgr.es/m/391415.1680268470@sss.pgh.pa.us
1 parentf0d65c0 commita2a0c7c

File tree

4 files changed

+88
-28
lines changed

4 files changed

+88
-28
lines changed

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

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4108,39 +4108,63 @@ width_bucket_float8(PG_FUNCTION_ARGS)
41084108

41094109
if (bound1<bound2)
41104110
{
4111-
/* In all cases, we'll add one at the end */
41124111
if (operand<bound1)
4113-
result=-1;
4112+
result=0;
41144113
elseif (operand >=bound2)
4115-
result=count;
4116-
elseif (!isinf(bound2-bound1))
41174114
{
4118-
/* Result of division is surely in [0,1], so this can't overflow */
4119-
result=count* ((operand-bound1) / (bound2-bound1));
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")));
41204119
}
41214120
else
41224121
{
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));
4122+
if (!isinf(bound2-bound1))
4123+
{
4124+
/* The quotient is surely in [0,1], so this can't overflow */
4125+
result=count* ((operand-bound1) / (bound2-bound1));
4126+
}
4127+
else
4128+
{
4129+
/*
4130+
* We get here if bound2 - bound1 overflows DBL_MAX. Since
4131+
* both bounds are finite, their difference can't exceed twice
4132+
* DBL_MAX; so we can perform the computation without overflow
4133+
* by dividing all the inputs by 2. That should be exact too,
4134+
* except in the case where a very small operand underflows to
4135+
* zero, which would have negligible impact on the result
4136+
* given such large bounds.
4137+
*/
4138+
result=count* ((operand /2-bound1 /2) / (bound2 /2-bound1 /2));
4139+
}
4140+
/* The quotient could round to 1.0, which would be a lie */
4141+
if (result >=count)
4142+
result=count-1;
4143+
/* Having done that, we can add 1 without fear of overflow */
4144+
result++;
41324145
}
41334146
}
41344147
elseif (bound1>bound2)
41354148
{
41364149
if (operand>bound1)
4137-
result=-1;
4150+
result=0;
41384151
elseif (operand <=bound2)
4139-
result=count;
4140-
elseif (!isinf(bound1-bound2))
4141-
result=count* ((bound1-operand) / (bound1-bound2));
4152+
{
4153+
if (pg_add_s32_overflow(count,1,&result))
4154+
ereport(ERROR,
4155+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
4156+
errmsg("integer out of range")));
4157+
}
41424158
else
4143-
result=count* ((bound1 /2-operand /2) / (bound1 /2-bound2 /2));
4159+
{
4160+
if (!isinf(bound1-bound2))
4161+
result=count* ((bound1-operand) / (bound1-bound2));
4162+
else
4163+
result=count* ((bound1 /2-operand /2) / (bound1 /2-bound2 /2));
4164+
if (result >=count)
4165+
result=count-1;
4166+
result++;
4167+
}
41444168
}
41454169
else
41464170
{
@@ -4150,10 +4174,5 @@ width_bucket_float8(PG_FUNCTION_ARGS)
41504174
result=0;/* keep the compiler quiet */
41514175
}
41524176

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-
41584177
PG_RETURN_INT32(result);
41594178
}

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1907,7 +1907,7 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
19071907
}
19081908

19091909
/*
1910-
*If'operand' isnot outsidethe bucket range, determine the correct
1910+
* 'operand' isinsidethe bucket range, so determine the correct
19111911
* bucket for it to go. The calculations performed by this function
19121912
* are derived directly from the SQL2003 spec. Note however that we
19131913
* multiply by count before dividing, to avoid unnecessary roundoff error.
@@ -1940,8 +1940,19 @@ compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
19401940
operand_var.dscale+count_var->dscale);
19411941
div_var(&operand_var,&bound2_var,result_var,
19421942
select_div_scale(&operand_var,&bound2_var), true);
1943-
add_var(result_var,&const_one,result_var);
1944-
floor_var(result_var,result_var);
1943+
1944+
/*
1945+
* Roundoff in the division could give us a quotient exactly equal to
1946+
* "count", which is too large. Clamp so that we do not emit a result
1947+
* larger than "count".
1948+
*/
1949+
if (cmp_var(result_var,count_var) >=0)
1950+
set_var_from_var(count_var,result_var);
1951+
else
1952+
{
1953+
add_var(result_var,&const_one,result_var);
1954+
floor_var(result_var,result_var);
1955+
}
19451956

19461957
free_var(&bound1_var);
19471958
free_var(&bound2_var);

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,31 @@ FROM generate_series(0, 110, 10) x;
14731473
110 | 0 | 0
14741474
(12 rows)
14751475

1476+
-- Another roundoff-error hazard
1477+
SELECT width_bucket(0, -1e100::numeric, 1, 10);
1478+
width_bucket
1479+
--------------
1480+
10
1481+
(1 row)
1482+
1483+
SELECT width_bucket(0, -1e100::float8, 1, 10);
1484+
width_bucket
1485+
--------------
1486+
10
1487+
(1 row)
1488+
1489+
SELECT width_bucket(1, 1e100::numeric, 0, 10);
1490+
width_bucket
1491+
--------------
1492+
10
1493+
(1 row)
1494+
1495+
SELECT width_bucket(1, 1e100::float8, 0, 10);
1496+
width_bucket
1497+
--------------
1498+
10
1499+
(1 row)
1500+
14761501
-- Check cases that could trigger overflow or underflow within the calculation
14771502
SELECT oper, low, high, cnt, width_bucket(oper, low, high, cnt)
14781503
FROM

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,11 @@ FROM generate_series(0, 110, 10) x;
909909
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;
912+
-- Another roundoff-error hazard
913+
SELECT width_bucket(0,-1e100::numeric,1,10);
914+
SELECT width_bucket(0,-1e100::float8,1,10);
915+
SELECT width_bucket(1, 1e100::numeric,0,10);
916+
SELECT width_bucket(1, 1e100::float8,0,10);
912917

913918
-- Check cases that could trigger overflow or underflow within the calculation
914919
SELECT oper, low, high, cnt, width_bucket(oper, low, high, cnt)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp