Обсуждение: Incorrect results from numeric round() and trunc()

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.

Regards,
Dean

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