Incorrect results from numeric round() and trunc()

On Sun, Jul 7, 2024, at 13:28, Dean Rasheed wrote:

The numeric round() and trunc() functions clamp the scale argument to
the range between +/- NUMERIC_MAX_RESULT_SCALE, which is +/- 2000.
That's a long way short of the actual allowed range of type numeric,
so they produce incorrect results when rounding/truncating more than
2000 digits before or after the decimal point. For example,
round(1e-5000, 5000) returns 0 instead of 1e-5000.

Attached is a patch fixing that, using the actual documented range of
type numeric.

I've also tidied up a bit by replacing all instances of SHRT_MAX with
a new constant NUMERIC_WEIGHT_MAX, whose name more accurately
describes the limit, as used in various other overflow checks.

In doing so, I also noticed a comment in power_var() which claimed
that ln_dweight could be as low as -SHRT_MAX (-32767), which is wrong.
It can only be as small as -NUMERIC_DSCALE_MAX (-16383), though that
doesn't affect the point being made in that comment.

I'd like to treat this as a bug-fix and back-patch it, since the
current behaviour is clearly broken.

Fix seems straightforward to me.
I agree it should be back-patched.

Regards,
Joel

On Sun, Jul 7, 2024, at 13:28, Dean Rasheed wrote:

I've also tidied up a bit by replacing all instances of SHRT_MAX with
a new constant NUMERIC_WEIGHT_MAX, whose name more accurately
describes the limit, as used in various other overflow checks.

Having thought a bit more on this, I think we probably need a
DEC_DIGITS sensitive definition of NUMERIC_WEIGHT_MAX,
since per spec the max range for numeric is 0x20000 (131072)
decimal digits.

Therefore, I think perhaps what we want is:

+#define NUMERIC_DSCALE_MIN                     0
+#define NUMERIC_WEIGHT_MAX                     ((0x20000/DEC_DIGITS)-1)
+#define NUMERIC_WEIGHT_MIN                     (-(NUMERIC_DSCALE_MAX+1)/DEC_DIGITS)

Maybe also 0x20000 (131072) should be a defined constant.

Regards,
Joel

On Mon, 8 Jul 2024 at 00:40, Joel Jacobson <joel@compiler.org> wrote:

On Sun, Jul 7, 2024, at 13:28, Dean Rasheed wrote:

I've also tidied up a bit by replacing all instances of SHRT_MAX with
a new constant NUMERIC_WEIGHT_MAX, whose name more accurately
describes the limit, as used in various other overflow checks.

Having thought a bit more on this, I think we probably need a
DEC_DIGITS sensitive definition of NUMERIC_WEIGHT_MAX,
since per spec the max range for numeric is 0x20000 (131072)
decimal digits.

No, the maximum weight is determined by the use of int16 to store the
weight. Therefore if you did reduce DEC_DIGITS to 1 or 2, the number
of decimal digits allowed before the decimal point would be reduced
too.

Regards,
Dean

On Mon, Jul 8, 2024, at 11:45, Dean Rasheed wrote:

On Mon, 8 Jul 2024 at 00:40, Joel Jacobson <joel@compiler.org> wrote:

On Sun, Jul 7, 2024, at 13:28, Dean Rasheed wrote:

I've also tidied up a bit by replacing all instances of SHRT_MAX with
a new constant NUMERIC_WEIGHT_MAX, whose name more accurately
describes the limit, as used in various other overflow checks.

Having thought a bit more on this, I think we probably need a
DEC_DIGITS sensitive definition of NUMERIC_WEIGHT_MAX,
since per spec the max range for numeric is 0x20000 (131072)
decimal digits.

No, the maximum weight is determined by the use of int16 to store the
weight. Therefore if you did reduce DEC_DIGITS to 1 or 2, the number
of decimal digits allowed before the decimal point would be reduced
too.

OK, that can actually be seen as a feature, especially since it's
of course more likely DEC_DIGITS could increase in the future
than decrease.

For example, let's say we would double it to 8,
then if NUMERIC_WEIGHT_MAX would still be 0x7FFF (32767),
then the maximum range for numeric would increase from 131072 to 262144
decimal digits allowed before the decimal point.

LGTM.

Regards,
Joel

On Mon, 8 Jul 2024 at 11:08, Joel Jacobson <joel@compiler.org> wrote:

LGTM.

Thanks for the review. I have pushed and back-patched this.

Regards,
Dean