Fix precision and rounding issues in money multiplication and division.

The cash_div_intX functions applied rint() to the result of the division.
That's not merely useless (because the result is already an integer) but
it causes precision loss for values larger than 2^52 or so, because of
the forced conversion to float8.

On the other hand, the cash_mul_fltX functions neglected to apply rint() to
their multiplication results, thus possibly causing off-by-one outputs.

Per C standard, arithmetic between any integral value and a float value is
performed in float format.  Thus, cash_mul_flt4 and cash_div_flt4 produced
answers good to only about six digits, even when the float value is exact.
We can improve matters noticeably by widening the float inputs to double.
(It's tempting to consider using "long double" arithmetic if available,
but that's probably too much of a stretch for a back-patched fix.)

Also, document that cash_div_intX operators truncate rather than round.

Per bug #14663 from Richard Pistole.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/22403.1495223615@sss.pgh.pa.us

diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 42b2bb71bb1c7be4eb88292a679bf4061d61e1a4..a32204900535f59e70331cbc72bda7312077f22a 100644 (file)
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -983,6 +983,11 @@ SELECT '52093.89'::money::numeric::float8;
    </para>
 
    <para>
+    Division of a <type>money</type> value by an integer value is performed
+    with truncation of the fractional part towards zero.  To get a rounded
+    result, divide by a floating-point value, or cast the <type>money</type>
+    value to <type>numeric</> before dividing and back to <type>money</type>
+    afterwards.  (The latter is preferable to avoid risking precision loss.)
     When a <type>money</type> value is divided by another <type>money</type>
     value, the result is <type>double precision</type> (i.e., a pure number,
     not money); the currency units cancel each other out in the division.

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 5cb086e50e6cafc00c3baeabe6ec005e2d419ec4..a170294b942091fe34a03065aa62870b1b7b76c9 100644 (file)
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -667,7 +667,7 @@ cash_mul_flt8(PG_FUNCTION_ARGS)
        float8          f = PG_GETARG_FLOAT8(1);
        Cash            result;
 
-       result = c * f;
+       result = rint(c * f);
        PG_RETURN_CASH(result);
 }
 
@@ -682,7 +682,7 @@ flt8_mul_cash(PG_FUNCTION_ARGS)
        Cash            c = PG_GETARG_CASH(1);
        Cash            result;
 
-       result = f * c;
+       result = rint(f * c);
        PG_RETURN_CASH(result);
 }
 
@@ -717,7 +717,7 @@ cash_mul_flt4(PG_FUNCTION_ARGS)
        float4          f = PG_GETARG_FLOAT4(1);
        Cash            result;
 
-       result = c * f;
+       result = rint(c * (float8) f);
        PG_RETURN_CASH(result);
 }
 
@@ -732,7 +732,7 @@ flt4_mul_cash(PG_FUNCTION_ARGS)
        Cash            c = PG_GETARG_CASH(1);
        Cash            result;
 
-       result = f * c;
+       result = rint((float8) f * c);
        PG_RETURN_CASH(result);
 }
 
@@ -753,7 +753,7 @@ cash_div_flt4(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
 
-       result = rint(c / f);
+       result = rint(c / (float8) f);
        PG_RETURN_CASH(result);
 }
 
@@ -802,7 +802,7 @@ cash_div_int8(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
 
-       result = rint(c / i);
+       result = c / i;
 
        PG_RETURN_CASH(result);
 }
@@ -854,7 +854,7 @@ cash_div_int4(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
 
-       result = rint(c / i);
+       result = c / i;
 
        PG_RETURN_CASH(result);
 }
@@ -904,7 +904,7 @@ cash_div_int2(PG_FUNCTION_ARGS)
                                (errcode(ERRCODE_DIVISION_BY_ZERO),
                                 errmsg("division by zero")));
 
-       result = rint(c / s);
+       result = c / s;
        PG_RETURN_CASH(result);
 }
 

diff --git a/src/test/regress/expected/money.out b/src/test/regress/expected/money.out
index 0cc69f925f50122f59b6462a2709c6a939b810f5..ab86595fc0219343da3374c11303e7d2d435d7a8 100644 (file)
--- a/src/test/regress/expected/money.out
+++ b/src/test/regress/expected/money.out
@@ -359,6 +359,56 @@ SELECT '92233720368547758.075'::money;
 ERROR:  value "92233720368547758.075" is out of range for type money
 LINE 1: SELECT '92233720368547758.075'::money;
                ^
+-- rounding vs. truncation in division
+SELECT '878.08'::money / 11::float8;
+ ?column? 
+----------
+   $79.83
+(1 row)
+
+SELECT '878.08'::money / 11::float4;
+ ?column? 
+----------
+   $79.83
+(1 row)
+
+SELECT '878.08'::money / 11::bigint;
+ ?column? 
+----------
+   $79.82
+(1 row)
+
+SELECT '878.08'::money / 11::int;
+ ?column? 
+----------
+   $79.82
+(1 row)
+
+SELECT '878.08'::money / 11::smallint;
+ ?column? 
+----------
+   $79.82
+(1 row)
+
+-- check for precision loss in division
+SELECT '90000000000000099.00'::money / 10::bigint;
+         ?column?          
+---------------------------
+ $9,000,000,000,000,009.90
+(1 row)
+
+SELECT '90000000000000099.00'::money / 10::int;
+         ?column?          
+---------------------------
+ $9,000,000,000,000,009.90
+(1 row)
+
+SELECT '90000000000000099.00'::money / 10::smallint;
+         ?column?          
+---------------------------
+ $9,000,000,000,000,009.90
+(1 row)
+
 -- Cast int4/int8/numeric to money
 SELECT 1234567890::money;
        money       

diff --git a/src/test/regress/sql/money.sql b/src/test/regress/sql/money.sql
index f5a92f2a6951595398967402a0ab87c61ac19fd4..37b9ecce1fc4fea57bfc79d41ef99e7d80892d9a 100644 (file)
--- a/src/test/regress/sql/money.sql
+++ b/src/test/regress/sql/money.sql
@@ -97,6 +97,18 @@ SELECT '92233720368547758.08'::money;
 SELECT '-92233720368547758.085'::money;
 SELECT '92233720368547758.075'::money;
 
+-- rounding vs. truncation in division
+SELECT '878.08'::money / 11::float8;
+SELECT '878.08'::money / 11::float4;
+SELECT '878.08'::money / 11::bigint;
+SELECT '878.08'::money / 11::int;
+SELECT '878.08'::money / 11::smallint;
+
+-- check for precision loss in division
+SELECT '90000000000000099.00'::money / 10::bigint;
+SELECT '90000000000000099.00'::money / 10::int;
+SELECT '90000000000000099.00'::money / 10::smallint;
+
 -- Cast int4/int8/numeric to money
 SELECT 1234567890::money;
 SELECT 12345678901234567::money;