Re: BUG #19826: corr()/regr_r2() of constant input: NULL serially, 1 with partial aggregation

On Mon, Nov 3, 2026 at 8:33 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Hm, the actual bug report hasn't come through yet, but I gather your
> point is that float8_regr_combine hasn't gotten the word about the
> efforts we've made in float8_regr_accum to avoid roundoff error for
> all-the-same inputs.

Right. It's not only corr(): with 300000 identical rows and a parallel
plan, regr_slope(y, x) gives -1.2e17 and covar_pop(x, y) -2e-08, where
the serial plan gives NULL and 0.

> Good point, but isn't it also missing the latter routine's recently-added
> logic for forcing outputs to NaN when necessary?

Yes. Once the Sxy correction is skipped for a constant input, the NaN
that 0 * Inf used to produce is gone. v2 forces Sxy to NaN in that case,
as float8_regr_accum does since 7774249d0. The combine function can't
see the inputs, so it tests Sx and Sy instead.

In practice a partial state that has seen an Inf or NaN already has
Sxy = NaN, so I couldn't get a different result from SQL either way, but
I don't think the combine function should depend on that.

> Also, TBH, the proposed comments read like AI slop; they don't
> address any of the questions I had on first reading the patch.

Rewritten. The comment now explains why dropping the term is right:
both means are exactly c, but Sx1 and Sx2 are rounded sums, so their
computed difference isn't zero and leaves Sxx slightly positive. The
commit message also mentions 649828769 and 7774249d0.

The regression test compares partitionwise aggregation against the
serial plan, for a constant input and for Inf/NaN in one partition.

One existing expected output changes: the direct float8_regr_combine()
call in aggregates.sql passes two states with the same common X (7) but
different means, so Sxx is now 400 rather than 6280. Such states can't
come from real input; I can change the test input instead if you prefer.

v2 attached; make check passes on master.

Regards,
Venu

On Tue, Nov 3, 2026 at 8:33 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Venu K <venu2805k(at)gmail(dot)com> writes:
> > Attached is a patch for master.
>
> Hm, the actual bug report hasn't come through yet, but I gather your
> point is that float8_regr_combine hasn't gotten the word about the
> efforts we've made in float8_regr_accum to avoid roundoff error for
> all-the-same inputs. Good point, but isn't it also missing the latter
> routine's recently-added logic for forcing outputs to NaN when
> necessary?
>
> Also, TBH, the proposed comments read like AI slop; they don't
> address any of the questions I had on first reading the patch.
> Some more effort seems appropriate there.
>
> regards, tom lane