Dean Rasheed <dean.a.rash...@gmail.com> writes:
> However, looking more closely, I think this function is more or less
> completely broken:

> 1). It doesn't work if log10val2 < 0, because then m < 0, and it
> doesn't multiply by the remainder. And it then throws an overflow
> error, because result.dscale comes out wrong when m < 0.

> 2). The result.dscale calculation is wrong if log10val2 is a multiple
> of DEC_DIGITS, causing it to drop the last 4 digits.

> 3). If the scaled-up dividend doesn't fit in an int64, the numeric
> computation breaks if log10val2 >= 10 due to integer overflow.

Ugh.

I'm not quite sure that it's worth expending code space on the
log10val2 < 0 case (compared to just "Assert(log10val2 >= 0").
On the other hand, it's not much extra code, and committing it now
might save somebody reinventing that logic in future.

I've not actually tested the patch, but it looks reasonable
by eyeball.

                        regards, tom lane


Reply via email to