Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Yeah, that makes sense. Thinking about it some more, its potential
> benefit becomes even less apparent at higher rscales because the cost
> of ln() relative to sqrt() will become larger -- the number of Taylor
> series terms required will grow roughly in proportion to the number of
> digits N, whereas the number of iterations sqrt() needs to do only
> grows like log(N) I think, because of it's quadratic convergence. So
> let's get rid of it.
OK, done that way. I committed this with that change and some other
mostly-cosmetic revisions. Notable non-cosmetic changes:
* I forced all the computed rscales to be at least 0, via code likelocal_rscale = Max(local_rscale,
NUMERIC_MIN_DISPLAY_SCALE);
You had done that in some places but not all, with the result that
functions like mul_var could be invoked with negative rscale and hence
produce outputs with negative dscale. This is not allowed according to
the comments in the code, and while it might accidentally fail to fail,
I do not trust the code to operate properly in that regime. It might be
worth revisiting the whole question of negative dscale with an eye to not
calculating digits we don't need, but I think that's material for a
completely separate patch.
* I moved some of the new regression test cases into numeric_big.sql.
While they don't add more than a couple hundred msec on my dev machine,
they're probably going to cost a lot more than that on the slower
buildfarm machines, and I'm not sure that they add enough benefit to be
worth running all the time. This code really shouldn't suffer from many
portability issues.
(I am going to go run numeric_big manually on all my own buildfarm
critters just to be sure, though.)
regards, tom lane
