Re: Modulus operator returns negative values / numeric

Paul Tillotson <pntil@xxxxxxxxxxx> · Thu, 26 May 2005 08:56:34 -0400

Tom Lane wrote:

Paul Tillotson <pntil@xxxxxxxxxxx> writes:

I don't think anyone wants to defend the negative modulus as such, but to fix it, we have to do one of these:

(1) Keep rounding division, but rewrite the numeric modulus operator to use a form of division that always rounds towards zero.

or 

(2) Give up rounding division in favor of truncating towards zero.

or (3) increase the calculation precision (rscale), as suggested by
Alvaro's message.

Possibly that cannot work, but I haven't seen a proof.

I don't think that will work.  Before switching round_var() to 
trunc_var() at the end of div_var(), I tried recompiling it to say

   div_var(var1, var2, &tmp, rscale + 1);

instead of

   div_var(var1, var2, &tmp, rscale);

Around line 4129 in mod_var().  (Which would perform the division with 
one extra decimal place when calculating a modulus.)  It fixed the case 
that Alvaro used as a test, but I was still able to get a negative 
modulus by trying other values.

I think that adding digits to rscale will cause the negative modulus to 
become more rare, but it will always be possible to do get it.  For 
example, 123456789012345678980 / 123 is

100371373180768844.6341 (rounded to 4 decimal places.) 

If you divide with no extra decimal places you get 45 at the tens' and 
ones' digits.  If you divide with one extra decimal place, you get 44.6, 
which is truncated to 44.

But suppose that dividing that gave you

100371373180768844.999997

In that case, you would need to work it to at least 6 extra places 
before truncation would actually give you the expected 44 rather than 
45, because even when working it to 5 decimal places, the carry 
propagation would eventually carry into the ones digit, changing the 4 
to a 5. 

In other words, no arbitrary number of extra decimal places when calling 
div_var() will be always sufficient to prevent rounding up at some other 
decimal place.

It looks like the "bug" can be easily fixed by changing the end of div_var where it says

	round_var(result, rscale);

to 
	trunc_var(result, scale);

I cannot believe that that won't create problems at least as bad as it
solves.  Have you even tried the regression tests on this?

			regards, tom lane

<sheepish grin>  No.  Can you tell me how to do that?

Paul

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
     joining column's datatypes do not match