[HACKERS] Precision and rounding fixes for money type - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | [HACKERS] Precision and rounding fixes for money type |
| Date | |
| Msg-id | 22403.1495223615@sss.pgh.pa.us Whole thread Raw |
| List | pgsql-hackers |
I looked a bit more carefully at cash.c in the wake of bug #14663, https://www.postgresql.org/message-id/20170519164653.29941.19098%40wrigleys.postgresql.org It seems to me that there are three different bugs in the multiplication and division operators: 1. As noted in the bug thread, applying rint() to the result of an integer division is useless, and it will cause precision loss if the 64-bit result exceeds 2^52 or thereabouts. We should drop it. 2. On the other hand, the cash-times-float operators really should apply rint() rather than allowing the default truncation behavior to happen when converting the float product to int64. 3. At least with my compiler (gcc 4.4.7), it seems that arithmetic between an int64 value and a float4 value is performed by casting the int64 to float4 and doing the arithmetic in float4. This results in really serious precision loss, since the result's only good to six digits or so. ISTM we'd be well advised to have the cash-and-float4 operators widen the float4 input to float8 and do the arithmetic in float8, so that they don't lose more precision than they have to. On modern machines there's unlikely to be any detectable speed difference. The attached patch rectifies these things and adds documentation about the truncation behavior of cash division. I propose to apply all of it to HEAD. What I'm less sure about is how much of it is a candidate to back-patch. I think point 1 (precision loss in what should be an exact integer operation) is a clear bug and we should back-patch it. But the other cases are not as open-and-shut; maybe we should just change those behaviors in HEAD. Thoughts? regards, tom lane diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 42b2bb7..a322049 100644 *** a/doc/src/sgml/datatype.sgml --- b/doc/src/sgml/datatype.sgml *************** SELECT '52093.89'::money::numeric::float *** 983,988 **** --- 983,993 ---- </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 5cb086e..a170294 100644 *** a/src/backend/utils/adt/cash.c --- b/src/backend/utils/adt/cash.c *************** cash_mul_flt8(PG_FUNCTION_ARGS) *** 667,673 **** float8 f = PG_GETARG_FLOAT8(1); Cash result; ! result = c * f; PG_RETURN_CASH(result); } --- 667,673 ---- float8 f = PG_GETARG_FLOAT8(1); Cash result; ! result = rint(c * f); PG_RETURN_CASH(result); } *************** flt8_mul_cash(PG_FUNCTION_ARGS) *** 682,688 **** Cash c = PG_GETARG_CASH(1); Cash result; ! result = f * c; PG_RETURN_CASH(result); } --- 682,688 ---- Cash c = PG_GETARG_CASH(1); Cash result; ! result = rint(f * c); PG_RETURN_CASH(result); } *************** cash_mul_flt4(PG_FUNCTION_ARGS) *** 717,723 **** float4 f = PG_GETARG_FLOAT4(1); Cash result; ! result = c * f; PG_RETURN_CASH(result); } --- 717,723 ---- float4 f = PG_GETARG_FLOAT4(1); Cash result; ! result = rint(c * (float8) f); PG_RETURN_CASH(result); } *************** flt4_mul_cash(PG_FUNCTION_ARGS) *** 732,738 **** Cash c = PG_GETARG_CASH(1); Cash result; ! result = f * c; PG_RETURN_CASH(result); } --- 732,738 ---- Cash c = PG_GETARG_CASH(1); Cash result; ! result = rint((float8) f * c); PG_RETURN_CASH(result); } *************** cash_div_flt4(PG_FUNCTION_ARGS) *** 753,759 **** (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = rint(c / f); PG_RETURN_CASH(result); } --- 753,759 ---- (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = rint(c / (float8) f); PG_RETURN_CASH(result); } *************** cash_div_int8(PG_FUNCTION_ARGS) *** 802,808 **** (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = rint(c / i); PG_RETURN_CASH(result); } --- 802,808 ---- (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = c / i; PG_RETURN_CASH(result); } *************** cash_div_int4(PG_FUNCTION_ARGS) *** 854,860 **** (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = rint(c / i); PG_RETURN_CASH(result); } --- 854,860 ---- (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = c / i; PG_RETURN_CASH(result); } *************** cash_div_int2(PG_FUNCTION_ARGS) *** 904,910 **** (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = rint(c / s); PG_RETURN_CASH(result); } --- 904,910 ---- (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); ! result = c / s; PG_RETURN_CASH(result); } diff --git a/src/test/regress/expected/money.out b/src/test/regress/expected/money.out index 0cc69f9..ab86595 100644 *** a/src/test/regress/expected/money.out --- b/src/test/regress/expected/money.out *************** SELECT '92233720368547758.075'::money; *** 359,364 **** --- 359,414 ---- 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 f5a92f2..37b9ecc 100644 *** a/src/test/regress/sql/money.sql --- b/src/test/regress/sql/money.sql *************** SELECT '92233720368547758.08'::money; *** 97,102 **** --- 97,114 ---- 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; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
pgsql-hackers by date: