Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c101d83

Browse files
committed
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
1 parent cb6a498 commit c101d83

File tree

4 files changed

+61
-8
lines changed

4 files changed

+61
-8
lines changed

doc/src/sgml/datatype.sgml

+5
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,11 @@ SELECT '52093.89'::money::numeric::float8;
977977
</para>
978978

979979
<para>
980+
Division of a <type>money</type> value by an integer value is performed
981+
with truncation of the fractional part towards zero. To get a rounded
982+
result, divide by a floating-point value, or cast the <type>money</type>
983+
value to <type>numeric</> before dividing and back to <type>money</type>
984+
afterwards. (The latter is preferable to avoid risking precision loss.)
980985
When a <type>money</type> value is divided by another <type>money</type>
981986
value, the result is <type>double precision</type> (i.e., a pure number,
982987
not money); the currency units cancel each other out in the division.

src/backend/utils/adt/cash.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ cash_mul_flt8(PG_FUNCTION_ARGS)
621621
float8 f = PG_GETARG_FLOAT8(1);
622622
Cash result;
623623

624-
result = c * f;
624+
result = rint(c * f);
625625
PG_RETURN_CASH(result);
626626
}
627627

@@ -636,7 +636,7 @@ flt8_mul_cash(PG_FUNCTION_ARGS)
636636
Cash c = PG_GETARG_CASH(1);
637637
Cash result;
638638

639-
result = f * c;
639+
result = rint(f * c);
640640
PG_RETURN_CASH(result);
641641
}
642642

@@ -671,7 +671,7 @@ cash_mul_flt4(PG_FUNCTION_ARGS)
671671
float4 f = PG_GETARG_FLOAT4(1);
672672
Cash result;
673673

674-
result = c * f;
674+
result = rint(c * (float8) f);
675675
PG_RETURN_CASH(result);
676676
}
677677

@@ -686,7 +686,7 @@ flt4_mul_cash(PG_FUNCTION_ARGS)
686686
Cash c = PG_GETARG_CASH(1);
687687
Cash result;
688688

689-
result = f * c;
689+
result = rint((float8) f * c);
690690
PG_RETURN_CASH(result);
691691
}
692692

@@ -707,7 +707,7 @@ cash_div_flt4(PG_FUNCTION_ARGS)
707707
(errcode(ERRCODE_DIVISION_BY_ZERO),
708708
errmsg("division by zero")));
709709

710-
result = rint(c / f);
710+
result = rint(c / (float8) f);
711711
PG_RETURN_CASH(result);
712712
}
713713

@@ -756,7 +756,7 @@ cash_div_int8(PG_FUNCTION_ARGS)
756756
(errcode(ERRCODE_DIVISION_BY_ZERO),
757757
errmsg("division by zero")));
758758

759-
result = rint(c / i);
759+
result = c / i;
760760

761761
PG_RETURN_CASH(result);
762762
}
@@ -808,7 +808,7 @@ cash_div_int4(PG_FUNCTION_ARGS)
808808
(errcode(ERRCODE_DIVISION_BY_ZERO),
809809
errmsg("division by zero")));
810810

811-
result = rint(c / i);
811+
result = c / i;
812812

813813
PG_RETURN_CASH(result);
814814
}
@@ -858,7 +858,7 @@ cash_div_int2(PG_FUNCTION_ARGS)
858858
(errcode(ERRCODE_DIVISION_BY_ZERO),
859859
errmsg("division by zero")));
860860

861-
result = rint(c / s);
861+
result = c / s;
862862
PG_RETURN_CASH(result);
863863
}
864864

src/test/regress/expected/money.out

+38
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,44 @@ SELECT * FROM money_data;
185185
$123.46
186186
(1 row)
187187

188+
-- rounding vs. truncation in division
189+
SELECT '878.08'::money / 11::float8;
190+
?column?
191+
----------
192+
$79.83
193+
(1 row)
194+
195+
SELECT '878.08'::money / 11::float4;
196+
?column?
197+
----------
198+
$79.83
199+
(1 row)
200+
201+
SELECT '878.08'::money / 11::int;
202+
?column?
203+
----------
204+
$79.82
205+
(1 row)
206+
207+
SELECT '878.08'::money / 11::smallint;
208+
?column?
209+
----------
210+
$79.82
211+
(1 row)
212+
213+
-- check for precision loss in division
214+
SELECT '90000000000000099.00'::money / 10::int;
215+
?column?
216+
---------------------------
217+
$9,000,000,000,000,009.90
218+
(1 row)
219+
220+
SELECT '90000000000000099.00'::money / 10::smallint;
221+
?column?
222+
---------------------------
223+
$9,000,000,000,000,009.90
224+
(1 row)
225+
188226
-- Cast int4/int8 to money
189227
SELECT 1234567890::money;
190228
money

src/test/regress/sql/money.sql

+10
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,16 @@ DELETE FROM money_data;
5757
INSERT INTO money_data VALUES ('$123.459');
5858
SELECT * FROM money_data;
5959

60+
-- rounding vs. truncation in division
61+
SELECT '878.08'::money / 11::float8;
62+
SELECT '878.08'::money / 11::float4;
63+
SELECT '878.08'::money / 11::int;
64+
SELECT '878.08'::money / 11::smallint;
65+
66+
-- check for precision loss in division
67+
SELECT '90000000000000099.00'::money / 10::int;
68+
SELECT '90000000000000099.00'::money / 10::smallint;
69+
6070
-- Cast int4/int8 to money
6171
SELECT 1234567890::money;
6272
SELECT 12345678901234567::money;

0 commit comments

Comments
 (0)