Re: [COMMITTERS] pgsql: Fix NUMERIC modulus to properly truncate - Mailing list pgsql-hackers

With no conclusion on this, I have added a TODO item:

* Add NUMERIC division operator that doesn't round?
 Currently NUMERIC _rounds_ the result to the specified precision. This means division can return a result that
multipliedby the divisor is greater than the dividend, e.g. this returns a value > 10:
 
   SELECT (10::numeric(2,0) / 6::numeric(2,0))::numeric(2,0) * 6;
 The positive modulus result returned by NUMERICs might be considered inaccurate, in one sense.


---------------------------------------------------------------------------

Bruce Momjian wrote:
> 
> Have we made any decision on whether the old/new NUMERIC %(mod) code was
> correct, and now to handle rounding?  Should we offer a non-rounding
> division operator for NUMERIC?
> 
> ---------------------------------------------------------------------------
> 
> Paul Tillotson wrote:
> > 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
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073