Re: [COMMITTERS] pgsql: Fix NUMERIC modulus to properly

Bruce Momjian wrote:

>Tom Lane wrote:
>
>
>>Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>>
>>
>>>>No, I don't think so.  It doesn't seem to be something that enough
>>>>people use to risk the change in behavior --- it might break something
>>>>that was working.  But, if folks want it backported we can do it.  It is
>>>>only a change to properly do modulus for numeric.
>>>>
>>>>
>>>Well, from my point of view it's an absolute mathematical error - i'd
>>>backport it.  I can't see anyone relying on it :)
>>>
>>>
>>Doesn't this patch break the basic theorem that
>>
>>    a = trunc(a / b) * b + (a mod b)
>>
>>?  If division rounds and mod doesn't, you've got pretty serious issues.
>>
>>
>
>Well, this is a good question.  In the equation above we assume '/' is
>an integer division.  The problem with NUMERIC when used with zero-scale
>operands is that the result is already _rounded_ to the nearest hole
>number before it gets to trunc(), and that is why we used to get
>negative modulus values.  I assume the big point is that we don't offer
>any way for users to get a NUMERIC division without rounding.
>
>With integers, we always round down to the nearest whole number on
>division;  float doesn't offer a modulus operator, and C doesn't support
>it either.
>
>We round NUMERICs to the specific scale because we want to give the most
>accurate value:
>
>    test=> select 100000000000000000000000::numeric(24,0) /
>    11::numeric(24,0);
>            ?column?
>    ------------------------
>     9090909090909090909091
>
>The actual values is:
>                                --
>     9090909090909090909090.90
>
>But the problem is that the equation at the top assumes the division is
>not rounded.  Should we supply a NUMERIC division operator that doesn't
>round?  integer doesn't need it, and float doesn't have the accurate
>precision needed for modulus operators.  The user could supply some
>digits in the division:
>
>    test=> select 100000000000000000000000::numeric(30,6) /
>    11::numeric(24,0);
>               ?column?
>    -------------------------------
>     9090909090909090909090.909091
>    (1 row)
>
>but there really is no _right_ value to prevent rounding (think
>0.9999999).  A non-rounding NUMERIC division would require duplicating
>numeric_div() but with a false for 'round', and adding operators.
>
>
>
I would prefer that division didn't round, as with integers.  You can
always calculate your result to 1 more decimal place and then round, but
there is no way to unround a rounded result.

Tom had asked whether PG passed the regression tests if we change the
round_var() to a trunc_var() at the end of the function div_var().

It does not pass, but I think that is because the regression test is
expecting that division will round up.  (Curiously, the regression test
for "numeric" passes, but the regression test for aggregation--sum() I
think--is the one that fails.)  I have attached the diffs here if anyone
is interested.

Regards,
Paul Tillotson

*** ./expected/aggregates.out    Sun May 29 19:58:43 2005
--- ./results/aggregates.out    Mon Jun  6 21:01:11 2005
***************
*** 10,16 ****
  SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100;
         avg_32
  ---------------------
!  32.6666666666666667
  (1 row)

  -- In 7.1, avg(float4) is computed using float8 arithmetic.
--- 10,16 ----
  SELECT avg(a) AS avg_32 FROM aggtest WHERE a < 100;
         avg_32
  ---------------------
!  32.6666666666666666
  (1 row)

  -- In 7.1, avg(float4) is computed using float8 arithmetic.

======================================================================

test boolean              ... ok
test char                 ... ok
test name                 ... ok
test varchar              ... ok
test text                 ... ok
test int2                 ... ok
test int4                 ... ok
test int8                 ... ok
test oid                  ... ok
test float4               ... ok
test float8               ... ok
test bit                  ... ok
test numeric              ... ok
test strings              ... ok
test numerology           ... ok
test point                ... ok
test lseg                 ... ok
test box                  ... ok
test path                 ... ok
test polygon              ... ok
test circle               ... ok
test date                 ... ok
test time                 ... ok
test timetz               ... ok
test timestamp            ... ok
test timestamptz          ... ok
test interval             ... ok
test abstime              ... ok
test reltime              ... ok
test tinterval            ... ok
test inet                 ... ok
test comments             ... ok
test oidjoins             ... ok
test type_sanity          ... ok
test opr_sanity           ... ok
test geometry             ... ok
test horology             ... ok
test insert               ... ok
test create_function_1    ... ok
test create_type          ... ok
test create_table         ... ok
test create_function_2    ... ok
test copy                 ... ok
test constraints          ... ok
test triggers             ... ok
test create_misc          ... ok
test create_aggregate     ... ok
test create_operator      ... ok
test create_index         ... ok
test inherit              ... ok
test vacuum               ... ok
test create_view          ... ok
test sanity_check         ... ok
test errors               ... ok
test select               ... ok
test select_into          ... ok
test select_distinct      ... ok
test select_distinct_on   ... ok
test select_implicit      ... ok
test select_having        ... ok
test subselect            ... ok
test union                ... ok
test case                 ... ok
test join                 ... ok
test aggregates           ... FAILED
test transactions         ... ok
test random               ... ok
test portals              ... ok
test arrays               ... ok
test btree_index          ... ok
test hash_index           ... ok
test update               ... ok
test namespace            ... ok
test privileges           ... ok
test misc                 ... ok
test select_views         ... ok
test portals_p2           ... ok
test rules                ... ok
test foreign_key          ... ok
test cluster              ... ok
test limit                ... ok
test plpgsql              ... ok
test copy2                ... ok
test temp                 ... ok
test domain               ... ok
test rangefuncs           ... ok
test prepare              ... ok
test without_oid          ... ok
test conversion           ... ok
test truncate             ... ok
test alter_table          ... ok
test sequence             ... ok
test polymorphism         ... ok
test rowtypes             ... ok
test stats                ... ok
test tablespace           ... ok