Dean Rasheed <dean.a.rash...@gmail.com> writes: > On 12 November 2015 at 21:01, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I started to look at this patch, and was immediately bemused by the >> comment in estimate_ln_weight:

> That's nonsense. The comment is perfectly correct. It's not saying the > logarithm is negative, it's saying that the *weight* of the logarithm > is negative. Ah, you're right --- I'd gotten confused about the distinction between ln(x) and ln(ln(x)). Nevermind ... Next question: in the additional range-reduction step you added to ln_var, why stop there, ie, what's the rationale for this magic number: if (Abs((x.weight + 1) * DEC_DIGITS) > 10) Seems like we arguably should do this whenever the weight isn't zero, so as to minimize the number of sqrt() steps. (Yes, I see the point about not getting into infinite recursion, but that only says that the threshold needs to be more than 10, not that it needs to be 10^10.) Also, it seems a little odd to put the recursive calculation of ln(10) where you did, rather than down where it's used, ie why not mul_var(result, &fact, result, local_rscale); ln_var(&const_ten, &ln_10, local_rscale); int64_to_numericvar((int64) pow_10, &ni); mul_var(&ln_10, &ni, &xx, local_rscale); add_var(result, &xx, result); round_var(result, rscale); As you have it, ln_10 will be calculated with possibly a smaller rscale than is used in this stanza. That might be all right but it seems dubious --- couldn't the lower-precision result leak into digits we care about? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers