On Sun, 4 Jul 2021 at 09:43, David Rowley <dgrowle...@gmail.com> wrote: > > On Sat, 3 Jul 2021 at 11:04, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > > Thinking about this more, I think it's best not to risk back-patching. > > It *might* be safe, but it's difficult to really be sure of that. The > > bug itself is pretty unlikely to ever happen in practice, hence the > > lack of prior complaints, and in fact I only found it by an > > examination of the code. So it doesn't seem to be worth the risk. > > That seems like good logic to me. Perhaps we can reconsider that > decision if users complain about it.
Thanks. Pushed to master only. I think the other part (avoiding overflows in numeric_mul) is fairly straightforward and uncontentious, so barring objections, I'll push and back-patch it in a couple of days or so. Regards, Dean
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c new file mode 100644 index bc71326..2a0f68f --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -233,6 +233,7 @@ struct NumericData */ #define NUMERIC_DSCALE_MASK 0x3FFF +#define NUMERIC_DSCALE_MAX NUMERIC_DSCALE_MASK #define NUMERIC_SIGN(n) \ (NUMERIC_IS_SHORT(n) ? \ @@ -2958,7 +2959,11 @@ numeric_mul_opt_error(Numeric num1, Nume * Unlike add_var() and sub_var(), mul_var() will round its result. In the * case of numeric_mul(), which is invoked for the * operator on numerics, * we request exact representation for the product (rscale = sum(dscale of - * arg1, dscale of arg2)). + * arg1, dscale of arg2)). If the exact result has more digits after the + * decimal point than can be stored in a numeric, we round it. Rounding + * after computing the exact result ensures that the final result is + * correctly rounded (rounding in mul_var() using a truncated product + * would not guarantee this). */ init_var_from_num(num1, &arg1); init_var_from_num(num2, &arg2); @@ -2966,6 +2971,9 @@ numeric_mul_opt_error(Numeric num1, Nume init_var(&result); mul_var(&arg1, &arg2, &result, arg1.dscale + arg2.dscale); + if (result.dscale > NUMERIC_DSCALE_MAX) + round_var(&result, NUMERIC_DSCALE_MAX); + res = make_result_opt_error(&result, have_error); free_var(&result); diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out new file mode 100644 index 4ad4851..385e963 --- a/src/test/regress/expected/numeric.out +++ b/src/test/regress/expected/numeric.out @@ -2145,6 +2145,12 @@ select 476999999999999999999999999999999 47699999999999999999999999999999999999999999999999999999999999999999999999999999999999985230000000000000000000000000000000000000000000000000000000000000000000000000000000000001 (1 row) +select trim_scale((0.1 - 2e-16383) * (0.1 - 3e-16383)); + trim_scale +------------ + 0.01 +(1 row) + -- -- Test some corner cases for division -- diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql new file mode 100644 index 3784c52..7e17c28 --- a/src/test/regress/sql/numeric.sql +++ b/src/test/regress/sql/numeric.sql @@ -1044,6 +1044,8 @@ select 477099999999999999999999999999999 select 4769999999999999999999999999999999999999999999999999999999999999999999999999999999999999 * 9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999; +select trim_scale((0.1 - 2e-16383) * (0.1 - 3e-16383)); + -- -- Test some corner cases for division --
