Fix scale clamping in numeric round() and trunc().

The numeric round() and trunc() functions clamp the scale argument to
the range between +/- NUMERIC_MAX_RESULT_SCALE (2000), which is much
smaller than the actual allowed range of type numeric. As a result,
they return incorrect results when asked to round/truncate more than
2000 digits before or after the decimal point.

Fix by using the correct upper and lower scale limits based on the
actual allowed (and documented) range of type numeric.

While at it, use the new NUMERIC_WEIGHT_MAX constant instead of
SHRT_MAX in all other overflow checks, and fix a comment thinko in
power_var() introduced by e54a758d24 -- the minimum value of
ln_dweight is -NUMERIC_DSCALE_MAX (-16383), not -SHRT_MAX, though this
doesn't affect the point being made in the comment, that the resulting
local_rscale value may exceed NUMERIC_MAX_DISPLAY_SCALE (1000).

Back-patch to all supported branches.

Dean Rasheed, reviewed by Joel Jacobson.

Discussion: https://postgr.es/m/CAEZATCXB%2BrDTuMjhK5ZxcouufigSc-X4tGJCBTMpZ3n%3DxxQuhg%40mail.gmail.com

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 5510a203b03c3d48e4ce9241bc0c0d690b285b20..57386aabdfed991d18ed21ef2cc58c28e02e322b 100644 (file)
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -249,6 +249,13 @@ struct NumericData
     | ((n)->choice.n_short.n_header & NUMERIC_SHORT_WEIGHT_MASK)) \
    : ((n)->choice.n_long.n_weight))
 
+/*
+ * Maximum weight of a stored Numeric value (based on the use of int16 for the
+ * weight in NumericLong).  Note that intermediate values held in NumericVar
+ * and NumericSumAccum variables may have much larger weights.
+ */
+#define NUMERIC_WEIGHT_MAX         PG_INT16_MAX
+
 /* ----------
  * NumericVar is the format we use for arithmetic.  The digit-array part
  * is the same as the NumericData storage format, but the header is more
@@ -1545,10 +1552,15 @@ numeric_round(PG_FUNCTION_ARGS)
        PG_RETURN_NUMERIC(duplicate_numeric(num));
 
    /*
-    * Limit the scale value to avoid possible overflow in calculations
+    * Limit the scale value to avoid possible overflow in calculations.
+    *
+    * These limits are based on the maximum number of digits a Numeric value
+    * can have before and after the decimal point, but we must allow for one
+    * extra digit before the decimal point, in case the most significant
+    * digit rounds up; we must check if that causes Numeric overflow.
     */
-   scale = Max(scale, -NUMERIC_MAX_RESULT_SCALE);
-   scale = Min(scale, NUMERIC_MAX_RESULT_SCALE);
+   scale = Max(scale, -(NUMERIC_WEIGHT_MAX + 1) * DEC_DIGITS - 1);
+   scale = Min(scale, NUMERIC_DSCALE_MAX);
 
    /*
     * Unpack the argument and round it at the proper digit position
@@ -1594,10 +1606,13 @@ numeric_trunc(PG_FUNCTION_ARGS)
        PG_RETURN_NUMERIC(duplicate_numeric(num));
 
    /*
-    * Limit the scale value to avoid possible overflow in calculations
+    * Limit the scale value to avoid possible overflow in calculations.
+    *
+    * These limits are based on the maximum number of digits a Numeric value
+    * can have before and after the decimal point.
     */
-   scale = Max(scale, -NUMERIC_MAX_RESULT_SCALE);
-   scale = Min(scale, NUMERIC_MAX_RESULT_SCALE);
+   scale = Max(scale, -(NUMERIC_WEIGHT_MAX + 1) * DEC_DIGITS);
+   scale = Min(scale, NUMERIC_DSCALE_MAX);
 
    /*
     * Unpack the argument and truncate it at the proper digit position
@@ -7276,7 +7291,7 @@ set_var_from_non_decimal_integer_str(const char *str, const char *cp, int sign,
                    add_var(dest, &tmp_var, dest);
 
                    /* Result will overflow if weight overflows int16 */
-                   if (dest->weight > SHRT_MAX)
+                   if (dest->weight > NUMERIC_WEIGHT_MAX)
                        goto out_of_range;
 
                    /* Begin a new group */
@@ -7313,7 +7328,7 @@ set_var_from_non_decimal_integer_str(const char *str, const char *cp, int sign,
                    add_var(dest, &tmp_var, dest);
 
                    /* Result will overflow if weight overflows int16 */
-                   if (dest->weight > SHRT_MAX)
+                   if (dest->weight > NUMERIC_WEIGHT_MAX)
                        goto out_of_range;
 
                    /* Begin a new group */
@@ -7350,7 +7365,7 @@ set_var_from_non_decimal_integer_str(const char *str, const char *cp, int sign,
                    add_var(dest, &tmp_var, dest);
 
                    /* Result will overflow if weight overflows int16 */
-                   if (dest->weight > SHRT_MAX)
+                   if (dest->weight > NUMERIC_WEIGHT_MAX)
                        goto out_of_range;
 
                    /* Begin a new group */
@@ -7386,7 +7401,7 @@ set_var_from_non_decimal_integer_str(const char *str, const char *cp, int sign,
    int64_to_numericvar(tmp, &tmp_var);
    add_var(dest, &tmp_var, dest);
 
-   if (dest->weight > SHRT_MAX)
+   if (dest->weight > NUMERIC_WEIGHT_MAX)
        goto out_of_range;
 
    dest->sign = sign;
@@ -11025,7 +11040,8 @@ power_var(const NumericVar *base, const NumericVar *exp, NumericVar *result)
    /*
     * Set the scale for the low-precision calculation, computing ln(base) to
     * around 8 significant digits.  Note that ln_dweight may be as small as
-    * -SHRT_MAX, so the scale may exceed NUMERIC_MAX_DISPLAY_SCALE here.
+    * -NUMERIC_DSCALE_MAX, so the scale may exceed NUMERIC_MAX_DISPLAY_SCALE
+    * here.
     */
    local_rscale = 8 - ln_dweight;
    local_rscale = Max(local_rscale, NUMERIC_MIN_DISPLAY_SCALE);
@@ -11133,7 +11149,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
        f = 0;                  /* result is 0 or 1 (weight 0), or error */
 
    /* overflow/underflow tests with fuzz factors */
-   if (f > (SHRT_MAX + 1) * DEC_DIGITS)
+   if (f > (NUMERIC_WEIGHT_MAX + 1) * DEC_DIGITS)
        ereport(ERROR,
                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                 errmsg("value overflows numeric format")));
@@ -11264,7 +11280,8 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
         * int16, the final result is guaranteed to overflow (or underflow, if
         * exp < 0), so we can give up before wasting too many cycles.
         */
-       if (base_prod.weight > SHRT_MAX || result->weight > SHRT_MAX)
+       if (base_prod.weight > NUMERIC_WEIGHT_MAX ||
+           result->weight > NUMERIC_WEIGHT_MAX)
        {
            /* overflow, unless neg, in which case result should be 0 */
            if (!neg)

diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 72f03c8a38a49b226957fb4e28c48733607627f0..f30ac236f528d5b2bf1db4c9424a14a0edac7a65 100644 (file)
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1346,6 +1346,108 @@ FROM generate_series(-5,5) AS t(i);
    5 |  -300000 |  -200000 |  -100000 |  100000 |  200000 |  300000
 (11 rows)
 
+-- Check limits of rounding before the decimal point
+SELECT round(4.4e131071, -131071) = 4e131071;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT round(4.5e131071, -131071) = 5e131071;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT round(4.5e131071, -131072); -- loses all digits
+ round 
+-------
+     0
+(1 row)
+
+SELECT round(5.5e131071, -131072); -- rounds up and overflows
+ERROR:  value overflows numeric format
+SELECT round(5.5e131071, -131073); -- loses all digits
+ round 
+-------
+     0
+(1 row)
+
+SELECT round(5.5e131071, -1000000); -- loses all digits
+ round 
+-------
+     0
+(1 row)
+
+-- Check limits of rounding after the decimal point
+SELECT round(5e-16383, 1000000) = 5e-16383;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT round(5e-16383, 16383) = 5e-16383;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT round(5e-16383, 16382) = 1e-16382;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT round(5e-16383, 16381) = 0;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Check limits of trunc() before the decimal point
+SELECT trunc(9.9e131071, -131071) = 9e131071;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT trunc(9.9e131071, -131072); -- loses all digits
+ trunc 
+-------
+     0
+(1 row)
+
+SELECT trunc(9.9e131071, -131073);  -- loses all digits
+ trunc 
+-------
+     0
+(1 row)
+
+SELECT trunc(9.9e131071, -1000000);  -- loses all digits
+ trunc 
+-------
+     0
+(1 row)
+
+-- Check limits of trunc() after the decimal point
+SELECT trunc(5e-16383, 1000000) = 5e-16383;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT trunc(5e-16383, 16383) = 5e-16383;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT trunc(5e-16383, 16382) = 0;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- Testing for width_bucket(). For convenience, we test both the
 -- numeric and float8 versions of the function in this file.
 -- errors

diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 83fc386333be68cdb3868e4db586738079ed7186..c86395209ab0c682736bd6c0844b6b55d5f3d6eb 100644 (file)
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -833,6 +833,31 @@ SELECT i as pow,
    round((2.5 * 10 ^ i)::numeric, -i)
 FROM generate_series(-5,5) AS t(i);
 
+-- Check limits of rounding before the decimal point
+SELECT round(4.4e131071, -131071) = 4e131071;
+SELECT round(4.5e131071, -131071) = 5e131071;
+SELECT round(4.5e131071, -131072); -- loses all digits
+SELECT round(5.5e131071, -131072); -- rounds up and overflows
+SELECT round(5.5e131071, -131073); -- loses all digits
+SELECT round(5.5e131071, -1000000); -- loses all digits
+
+-- Check limits of rounding after the decimal point
+SELECT round(5e-16383, 1000000) = 5e-16383;
+SELECT round(5e-16383, 16383) = 5e-16383;
+SELECT round(5e-16383, 16382) = 1e-16382;
+SELECT round(5e-16383, 16381) = 0;
+
+-- Check limits of trunc() before the decimal point
+SELECT trunc(9.9e131071, -131071) = 9e131071;
+SELECT trunc(9.9e131071, -131072); -- loses all digits
+SELECT trunc(9.9e131071, -131073);  -- loses all digits
+SELECT trunc(9.9e131071, -1000000);  -- loses all digits
+
+-- Check limits of trunc() after the decimal point
+SELECT trunc(5e-16383, 1000000) = 5e-16383;
+SELECT trunc(5e-16383, 16383) = 5e-16383;
+SELECT trunc(5e-16383, 16382) = 0;
+
 -- Testing for width_bucket(). For convenience, we test both the
 -- numeric and float8 versions of the function in this file.