From: | "Joel Jacobson" <joel(at)compiler(dot)org> |
---|---|
To: | "Dean Rasheed" <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Optimize mul_var() for var1ndigits >= 8 |
Date: | 2024-08-12 10:47:08 |
Message-ID: | aae22790-bc95-4b81-b04b-a30cfdb76a6c@app.fastmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
numeric_mul_rscale.txt | text/plain | 5.3 KB |
plausible_rscale_adjustments.csv | text/csv | 24.0 KB |
test-mul_var-init.sql | application/octet-stream | 975 bytes |
test-mul_var-verify.sql | application/octet-stream | 448 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Aleksander Alekseev | 2024-08-12 10:55:04 | Re: PostgreSQL's approach to assertion usage: seeking best practices |
Previous Message | John Naylor | 2024-08-12 10:46:22 | Re: ECPG cleanup and fix for clang compile-time problem |