Thread

  1. Re: [PATCH] Fix overflow and underflow in regr_r2()

    Chengpeng Yan <chengpeng_yan@outlook.com> — 2026-05-23T01:14:09Z

    Hi,
    
    > On May 16, 2026, at 17:39, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
    > 
    >> corr() already has a stabilized calculation for the same Sxx * Syy
    >> denominator scale. This patch factors that into a helper and lets
    >> regr_r2() use it as a fallback when one of its direct products has
    >> rounded to zero or infinity. Otherwise, regr_r2() keeps the existing
    >> direct formula.
    > 
    > The comments need work -- in particular float8_regr_r2() needs a
    > comment explaining the new overflow/underflow checks, similar to the
    > comment in float8_corr(). In fact, doing that, I think it's preferable
    > to just keep this change local to float8_regr_r2(), rather than
    > refactoring into a helper function for just a few lines of code.
    > 
    > This new check in float8_regr_r2():
    > 
    > +   if (Sxy == 0 && !isnan(Sxx) && !isnan(Syy))
    > +       PG_RETURN_FLOAT8(0.0);
    > 
    > seems pointless. It's optimising for a special case that will very
    > rarely occur in practice, and which is handled fine by the general
    > code. We don't want to slow down the common code path for such rare
    > special cases.
    > 
    > I noticed that this new overflow test case:
    > 
    > +SELECT regr_r2(1e154::float8 * g, 1e154::float8 * g)
    > +  FROM generate_series(1, 2) g;
    > + regr_r2
    > +---------
    > +       1
    > +(1 row)
    > 
    > only produces 1 because it's run with a reduced extra_float_digits
    > value. I think it's better to use the test values "1e100 + g * 1e95",
    > which still trigger the overflow on HEAD, but more reliably produce 1,
    > regardless of the extra_float_digits setting, making it less likely
    > that there will be variations between platforms. That's also more
    > consistent with the other nearby test cases.
    
    Thanks for the thorough review and feedback — I learned a lot from it!
    
    > Attached is a v2 patch with those changes, plus a little more tidying
    > up of the regression tests.
    
    v2 LGTM. Thanks for the updates and test cleanup.
    
    --
    Best regards,
    Chengpeng Yan