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
