Re: Optimize mul_var() for var1ndigits >= 8

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

In response to

Responses

Browse pgsql-hackers by date

  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