On Sun, Aug 11, 2024, at 22:04, Joel Jacobson wrote:
>> Attachments:
>> * v4-0001-Extend-mul_var_short-to-5-and-6-digit-inputs.patch
>> * v4-0002-Optimise-numeric-multiplication-using-base-NBASE-.patch
>
> I've reviewed and tested both patches and think they are ready to be committed.
In addition, I've also tested reduced rscale specifically, due to what you wrote earlier:
> 2). Attempt to fix the formulae incorporating maxdigits mentioned
> above. This part really made my brain hurt, and I'm still not quite
> sure that I've got it right. In particular, it needs double-checking
> to ensure that it's not losing accuracy in the reduced-rscale case.
To test if there are any differences that actually matter in the result,
I patched mul_var to log what combinations that occur when running
the test suite:
```
if (rscale != var1->dscale + var2->dscale)
{
printf("NUMERIC_REDUCED_RSCALE %d,%d,%d,%d,%d\n", var1ndigits, var2ndigits, var1->dscale, var2->dscale, rscale
-(var1->dscale + var2->dscale));
}
```
I also added a SQL-callable numeric_mul_rscale(var1, var2, rscale_adjustment) function,
to be able to check for differences for the reduced rscale combinations.
I then ran the test suite against my db and extracted the seen combinations:
```
make installcheck
grep -E "^NUMERIC_REDUCED_RSCALE \d+,\d+,\d+,\d+,-\d+$" logfile | sort -u | awk '{print $2}' >
plausible_rscale_adjustments.csv
```
This test didn't produce any differences between HEAD and the two patches applied.
% psql -f test-mul_var-verify.sql
CREATE TABLE
COPY 1413
var1ndigits | var2ndigits | var1dscale | var2dscale | rscale_adjustment | var1 | var2 | expected | numeric_mul_rscale
-------------+-------------+------------+------------+-------------------+------+------+----------+--------------------
(0 rows)
Attaching patch as .txt to not confuse cfbot.
Regards,
Joel
