bvolpato opened a new pull request, #24953:
URL: https://github.com/apache/datafusion/pull/24953
## Which issue does this PR close?
No linked issue. The regression below reproduces the problem on current
upstream.
## Rationale for this change
Grouped CORR uses raw sums whose subtraction loses precision when values
have a large common offset. It can return a negative correlation for a column
correlated with itself:
```sql
SELECT g, corr(x, x)
FROM (VALUES
(1, 1000000000.0),
(1, 1000000007.0),
(1, 1000000015.0),
(2, 1.0)
) AS t(g, x)
GROUP BY g;
```
Group 1 can return `-1` instead of `1`.
## What changes are included in this PR?
- Use paired Welford updates and centered moments for grouped correlation.
- Merge partial states using weighted mean differences, applying weights
before multiplying deltas to avoid avoidable intermediate overflow.
- Match the scalar CORR state layout across update, merge, singleton
conversion, and preserving/draining emission.
- Add regressions for large offsets, multiple batch sizes, scalar/grouped
state interchange, and large-range merges, plus a grouped self-correlation case
in `aggregate.slt`.
## What is the testing strategy for this PR?
- The large-offset and scalar/grouped state compatibility tests fail against
upstream production code at `35f58f53cde2f634c21f6370e385cf1d7e9bc55c` and pass
with the fix.
- `cargo test --locked -p datafusion-functions-aggregate --lib`: 210 passed.
- `cargo fmt --all -- --check` and `git diff --check` passed.
- Draft pending all-target/all-feature Clippy, the extended workspace suite,
and execution of `aggregate.slt`.
- The H2O groupby q09 benchmark exercises CORR. Comparison against main is
still pending.
## Are there any user-facing changes?
Grouped CORR produces stable results for the covered large-offset inputs.
Floating-point results can differ in their final bits. SQL signatures and
output types remain unchanged.
## Downsides
Centered updates add arithmetic, including per-row divisions. The grouped
intermediate state representation changes from raw sums to centered moments, so
partial states produced by the old implementation cannot be mixed with the new
representation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
