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

Commit7538708

Browse files
committed
Avoid gratuitous inaccuracy in numeric width_bucket().
Multiply before dividing, not the reverse, so that cases that shouldproduce exact results do produce exact results. (width_bucket_float8got this right already.) Even when the result is inexact, this avoidsmaking it more inexact, since only the division step introduces anyimprecision.While at it, fix compute_bucket() to not uselessly repeat the signcheck already done by its caller, and avoid duplicating themultiply/divide steps by adjusting variable usage.Per complaint from Martin Visser. Although this seems like a bug fix,I'm hesitant to risk changing width_bucket()'s results in stablebranches, so no back-patch.Discussion:https://postgr.es/m/6FA5117D-6AED-4656-8FEF-B74AC18FAD85@brytlyt.com
1 parent8ce423b commit7538708

File tree

3 files changed

+65
-15
lines changed

3 files changed

+65
-15
lines changed

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

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,8 @@ static void round_var(NumericVar *var, int rscale);
592592
staticvoidtrunc_var(NumericVar*var,intrscale);
593593
staticvoidstrip_var(NumericVar*var);
594594
staticvoidcompute_bucket(Numericoperand,Numericbound1,Numericbound2,
595-
constNumericVar*count_var,NumericVar*result_var);
595+
constNumericVar*count_var,boolreversed_bounds,
596+
NumericVar*result_var);
596597

597598
staticvoidaccum_sum_add(NumericSumAccum*accum,constNumericVar*var1);
598599
staticvoidaccum_sum_rescale(NumericSumAccum*accum,constNumericVar*val);
@@ -1752,8 +1753,8 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
17521753
elseif (cmp_numerics(operand,bound2) >=0)
17531754
add_var(&count_var,&const_one,&result_var);
17541755
else
1755-
compute_bucket(operand,bound1,bound2,
1756-
&count_var,&result_var);
1756+
compute_bucket(operand,bound1,bound2,&count_var, false,
1757+
&result_var);
17571758
break;
17581759

17591760
/* bound1 > bound2 */
@@ -1763,8 +1764,8 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
17631764
elseif (cmp_numerics(operand,bound2) <=0)
17641765
add_var(&count_var,&const_one,&result_var);
17651766
else
1766-
compute_bucket(operand,bound1,bound2,
1767-
&count_var,&result_var);
1767+
compute_bucket(operand,bound1,bound2,&count_var, true,
1768+
&result_var);
17681769
break;
17691770
}
17701771

@@ -1783,11 +1784,13 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
17831784
/*
17841785
* If 'operand' is not outside the bucket range, determine the correct
17851786
* bucket for it to go. The calculations performed by this function
1786-
* are derived directly from the SQL2003 spec.
1787+
* are derived directly from the SQL2003 spec. Note however that we
1788+
* multiply by count before dividing, to avoid unnecessary roundoff error.
17871789
*/
17881790
staticvoid
17891791
compute_bucket(Numericoperand,Numericbound1,Numericbound2,
1790-
constNumericVar*count_var,NumericVar*result_var)
1792+
constNumericVar*count_var,boolreversed_bounds,
1793+
NumericVar*result_var)
17911794
{
17921795
NumericVarbound1_var;
17931796
NumericVarbound2_var;
@@ -1797,23 +1800,21 @@ compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
17971800
init_var_from_num(bound2,&bound2_var);
17981801
init_var_from_num(operand,&operand_var);
17991802

1800-
if (cmp_var(&bound1_var,&bound2_var)<0)
1803+
if (!reversed_bounds)
18011804
{
18021805
sub_var(&operand_var,&bound1_var,&operand_var);
18031806
sub_var(&bound2_var,&bound1_var,&bound2_var);
1804-
div_var(&operand_var,&bound2_var,result_var,
1805-
select_div_scale(&operand_var,&bound2_var), true);
18061807
}
18071808
else
18081809
{
18091810
sub_var(&bound1_var,&operand_var,&operand_var);
1810-
sub_var(&bound1_var,&bound2_var,&bound1_var);
1811-
div_var(&operand_var,&bound1_var,result_var,
1812-
select_div_scale(&operand_var,&bound1_var), true);
1811+
sub_var(&bound1_var,&bound2_var,&bound2_var);
18131812
}
18141813

1815-
mul_var(result_var,count_var,result_var,
1816-
result_var->dscale+count_var->dscale);
1814+
mul_var(&operand_var,count_var,&operand_var,
1815+
operand_var.dscale+count_var->dscale);
1816+
div_var(&operand_var,&bound2_var,result_var,
1817+
select_div_scale(&operand_var,&bound2_var), true);
18171818
add_var(result_var,&const_one,result_var);
18181819
floor_var(result_var,result_var);
18191820

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,46 @@ SELECT width_bucket('Infinity'::float8, 1, 10, 10),
13851385
(1 row)
13861386

13871387
DROP TABLE width_bucket_test;
1388+
-- Simple test for roundoff error when results should be exact
1389+
SELECT x, width_bucket(x::float8, 10, 100, 9) as flt,
1390+
width_bucket(x::numeric, 10, 100, 9) as num
1391+
FROM generate_series(0, 110, 10) x;
1392+
x | flt | num
1393+
-----+-----+-----
1394+
0 | 0 | 0
1395+
10 | 1 | 1
1396+
20 | 2 | 2
1397+
30 | 3 | 3
1398+
40 | 4 | 4
1399+
50 | 5 | 5
1400+
60 | 6 | 6
1401+
70 | 7 | 7
1402+
80 | 8 | 8
1403+
90 | 9 | 9
1404+
100 | 10 | 10
1405+
110 | 10 | 10
1406+
(12 rows)
1407+
1408+
SELECT x, width_bucket(x::float8, 100, 10, 9) as flt,
1409+
width_bucket(x::numeric, 100, 10, 9) as num
1410+
FROM generate_series(0, 110, 10) x;
1411+
x | flt | num
1412+
-----+-----+-----
1413+
0 | 10 | 10
1414+
10 | 10 | 10
1415+
20 | 9 | 9
1416+
30 | 8 | 8
1417+
40 | 7 | 7
1418+
50 | 6 | 6
1419+
60 | 5 | 5
1420+
70 | 4 | 4
1421+
80 | 3 | 3
1422+
90 | 2 | 2
1423+
100 | 1 | 1
1424+
110 | 0 | 0
1425+
(12 rows)
1426+
1427+
--
13881428
-- TO_CHAR()
13891429
--
13901430
SELECT '' AS to_char_1, to_char(val, '9G999G999G999G999G999')

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,15 @@ SELECT width_bucket('Infinity'::float8, 1, 10, 10),
888888

889889
DROPTABLE width_bucket_test;
890890

891+
-- Simple test for roundoff error when results should be exact
892+
SELECT x, width_bucket(x::float8,10,100,9)as flt,
893+
width_bucket(x::numeric,10,100,9)as num
894+
FROM generate_series(0,110,10) x;
895+
SELECT x, width_bucket(x::float8,100,10,9)as flt,
896+
width_bucket(x::numeric,100,10,9)as num
897+
FROM generate_series(0,110,10) x;
898+
899+
--
891900
-- TO_CHAR()
892901
--
893902
SELECT''AS to_char_1, to_char(val,'9G999G999G999G999G999')

0 commit comments

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