From d761fe2182d9e26f9483e4b7ac303e38bfbd7a24 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 21 May 2017 13:05:16 -0400 Subject: [PATCH] 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 --- doc/src/sgml/datatype.sgml | 5 +++ src/backend/utils/adt/cash.c | 16 ++++----- src/test/regress/expected/money.out | 50 +++++++++++++++++++++++++++++ src/test/regress/sql/money.sql | 12 +++++++ 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 42b2bb71bb..a322049005 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -983,6 +983,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 5cb086e50e..a170294b94 100644 --- 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 0cc69f925f..ab86595fc0 100644 --- 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 f5a92f2a69..37b9ecce1f 100644 --- 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; -- 2.40.0