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 151a49037e417f4e79af00d6eb08b917cc25aa6a..52b5deb98735af9f9cde64dd061d181c55264d0d 100644 (file)
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -952,6 +952,11 @@ SELECT '52093.89'::money::numeric::float8;
    
 
    
+    Division of a money 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 money
+    value to numeric before dividing and back to money
+    afterwards.  (The latter is preferable to avoid risking precision loss.)
     When a money value is divided by another money
     value, the result is double precision (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 82551c5f30e6b6b4a40bdff891986fd92499ef9d..2630c8ee8677950eb94c84c76551da453a9e0c62 100644 (file)
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -623,7 +623,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);
 }
 
@@ -638,7 +638,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);
 }
 
@@ -673,7 +673,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);
 }
 
@@ -688,7 +688,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);
 }
 
@@ -709,7 +709,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);
 }
 
@@ -758,7 +758,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);
 }
@@ -810,7 +810,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);
 }
@@ -860,7 +860,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 538235c4cc26820660e95fc88474f0bbbc197c45..efb48f1acd6b3fe6b4237232cd31da148433748d 100644 (file)
--- a/src/test/regress/expected/money.out
+++ b/src/test/regress/expected/money.out
@@ -185,6 +185,44 @@ SELECT * FROM money_data;
  $123.46
 (1 row)
 
+-- 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::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::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 to money
 SELECT 1234567890::money;
        money       

diff --git a/src/test/regress/sql/money.sql b/src/test/regress/sql/money.sql
index 09b9476b706172f2e2d1758e95f8c684029f06dc..f0db5fa4322abdd075d82390556ca974eeb993f6 100644 (file)
--- a/src/test/regress/sql/money.sql
+++ b/src/test/regress/sql/money.sql
@@ -57,6 +57,16 @@ DELETE FROM money_data;
 INSERT INTO money_data VALUES ('$123.459');
 SELECT * FROM money_data;
 
+-- rounding vs. truncation in division
+SELECT '878.08'::money / 11::float8;
+SELECT '878.08'::money / 11::float4;
+SELECT '878.08'::money / 11::int;
+SELECT '878.08'::money / 11::smallint;
+
+-- check for precision loss in division
+SELECT '90000000000000099.00'::money / 10::int;
+SELECT '90000000000000099.00'::money / 10::smallint;
+
 -- Cast int4/int8 to money
 SELECT 1234567890::money;
 SELECT 12345678901234567::money;