database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-10-13 18:28:01 +03:00
Code Activity

Avoid gratuitous inaccuracy in numeric width_bucket().

Browse Source 
Multiply before dividing, not the reverse, so that cases that should
produce exact results do produce exact results.  (width_bucket_float8
got this right already.)  Even when the result is inexact, this avoids
making it more inexact, since only the division step introduces any
imprecision.

While at it, fix compute_bucket() to not uselessly repeat the sign
check already done by its caller, and avoid duplicating the
multiply/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 stable
branches, so no back-patch.

Discussion: https://postgr.es/m/6FA5117D-6AED-4656-8FEF-B74AC18FAD85@brytlyt.com
This commit is contained in:
Tom Lane
2020-10-08 13:06:27 -04:00
parent 8ce423b191
commit 7538708394
3 changed files with 65 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
src/backend/utils/adt/numeric.c
View File

@@ -592,7 +592,8 @@ static void round_var(NumericVar *var, int rscale);
static void trunc_var(NumericVar *var, int rscale);
static void strip_var(NumericVar *var);
static void compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
const NumericVar *count_var, NumericVar *result_var);
const NumericVar *count_var, bool reversed_bounds,
NumericVar *result_var);
static void accum_sum_add(NumericSumAccum *accum, const NumericVar *var1);
static void accum_sum_rescale(NumericSumAccum *accum, const NumericVar *val);
@@ -1752,8 +1753,8 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
else if (cmp_numerics(operand, bound2) >= 0)
add_var(&count_var, &const_one, &result_var);
else
compute_bucket(operand, bound1, bound2,
&count_var, &result_var);
compute_bucket(operand, bound1, bound2, &count_var, false,
&result_var);
break;
/* bound1 > bound2 */
@@ -1763,8 +1764,8 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
else if (cmp_numerics(operand, bound2) <= 0)
add_var(&count_var, &const_one, &result_var);
else
compute_bucket(operand, bound1, bound2,
&count_var, &result_var);
compute_bucket(operand, bound1, bound2, &count_var, true,
&result_var);
break;
}
@@ -1783,11 +1784,13 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
/*
* If 'operand' is not outside the bucket range, determine the correct
* bucket for it to go. The calculations performed by this function
* are derived directly from the SQL2003 spec.
* are derived directly from the SQL2003 spec. Note however that we
* multiply by count before dividing, to avoid unnecessary roundoff error.
*/
static void
compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
const NumericVar *count_var, NumericVar *result_var)
const NumericVar *count_var, bool reversed_bounds,
NumericVar *result_var)
{
NumericVar bound1_var;
NumericVar bound2_var;
@@ -1797,23 +1800,21 @@ compute_bucket(Numeric operand, Numeric bound1, Numeric bound2,
init_var_from_num(bound2, &bound2_var);
init_var_from_num(operand, &operand_var);
if (cmp_var(&bound1_var, &bound2_var) < 0)
if (!reversed_bounds)
{
sub_var(&operand_var, &bound1_var, &operand_var);
sub_var(&bound2_var, &bound1_var, &bound2_var);
div_var(&operand_var, &bound2_var, result_var,
select_div_scale(&operand_var, &bound2_var), true);
}
else
{
sub_var(&bound1_var, &operand_var, &operand_var);
sub_var(&bound1_var, &bound2_var, &bound1_var);
div_var(&operand_var, &bound1_var, result_var,
select_div_scale(&operand_var, &bound1_var), true);
sub_var(&bound1_var, &bound2_var, &bound2_var);
}
mul_var(result_var, count_var, result_var,
result_var->dscale + count_var->dscale);
mul_var(&operand_var, count_var, &operand_var,
operand_var.dscale + count_var->dscale);
div_var(&operand_var, &bound2_var, result_var,
select_div_scale(&operand_var, &bound2_var), true);
add_var(result_var, &const_one, result_var);
floor_var(result_var, result_var);