Feedback-ID: ic6394509:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-566-g3812ddbbc-fm-20240627.001-g3812ddbb
MIME-Version: 1.0
Message-Id: <9c0e3469-13e6-41c4-86ec-4f3f01c66e47@app.fastmail.com>
Date: Wed, 03 Jul 2024 09:35:07 +0200
From: "Joel Jacobson" <joel@compiler.org>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: numeric.c: Should MUL_GUARD_DIGITS be increased from 2 to 3?
Content-Type: text/plain
Archived-At: <https://www.postgresql.org/message-id/9c0e3469-13e6-41c4-86ec-4f3f01c66e47%40app.fastmail.com>
Precedence: bulk

Hello hackers,

I have discovered a peculiar behavior in mul_var() when it is called with
rscale=0, but the input variables have many decimal digits, resulting in a
product with a .5 decimal part. Given that no decimals are requested by the
caller, I would expect the result to be rounded up. However, it is instead
truncated or rounded down.

To investigate this further, I added an SQL-callable function,
numeric_mul_patched(), which takes rscale_adjustment as a third input parameter.
This allows calling mul_var() with varying rscale values.

Here is an example demonstrating the issue:

SELECT 5.51574061794 * 0.99715659165;
5.5000571150105152442010 -- exact result

SELECT numeric_mul_patched(5.51574061794,0.99715659165,-22);

The output debug information before and after modifying MUL_GUARD_DIGITS
from 2 to 3 is as follows:

-#define MUL_GUARD_DIGITS       2
+#define MUL_GUARD_DIGITS       3

-make_result(): NUMERIC w=0 d=11 POS 0005 5157 4061 7940
+make_result(): NUMERIC w=0 d=11 POS 0005 5157 4061 7940
-make_result(): NUMERIC w=-1 d=11 POS 9971 5659 1650
+make_result(): NUMERIC w=-1 d=11 POS 9971 5659 1650
-before round_var: VAR w=1 d=0 POS 0000 0005 4999 8742
+before round_var: VAR w=1 d=0 POS 0000 0005 5000 5710 3944
-after round_var: VAR w=1 d=0 POS 0000 0005
+after round_var: VAR w=1 d=0 POS 0000 0006
-make_result(): NUMERIC w=0 d=0 POS 0005
+make_result(): NUMERIC w=0 d=0 POS 0006
 numeric_mul_patched
---------------------
-                   5
+                   6
(1 row)

As shown above, changing MUL_GUARD_DIGITS from 2 to 3 corrects the rounding
error, ensuring the correct result of 6 instead of 5.

Although this is likely only a potential issue as current callers of mul_var()
don't use it in this specific way, it would be beneficial to fix it for
correctness and ensure mul_var() adheres to its contract.

I encountered this issue while working on an optimization of mul_var() in a
different thread. Initially, I thought there was an error in my code due to
the different result, but I later realized it was a rounding error in the
original code.

Not submitting a patch yet, since I might have misunderstood something here.
Maybe this is all fine after all. Guidance appreciated.

Regards,
Joel