ulysses-you opened a new pull request, #57460:
URL: https://github.com/apache/spark/pull/57460

   ### What changes were proposed in this pull request?
   
   The `merge` expressions of `PearsonCorrelation` (`Corr`), `Covariance`, and 
`CentralMomentAgg` overflow to `Infinity` and then to `NaN` via `Infinity * 0` 
when one side is an empty aggregation buffer whose counterpart holds a large 
average.
   
   Concretely, the merge computes terms like `delta * deltaN * n1 * n2` (and 
the `dx * dxN`, `dy * dyN` variants). When one buffer is empty (`n1 == 0` or 
`n2 == 0`), that term is mathematically zero, but `delta * deltaN` can overflow 
to `Infinity` *before* being multiplied by the zero count, and `Infinity * 0 = 
NaN` corrupts the merged moments.
   
   This PR forces the `delta`/`dx`/`dy` terms to `0.0` when either side is 
empty, and sets the merged average directly to the non-empty side's average, so 
the empty-side terms vanish cleanly. This mirrors the guard the other 
aggregates already apply.
   
   This is extracted from #57363 as a standalone correctness fix, independent 
of the `spark.sql.execution.replaceHashWithSortAgg` default flip, so it can be 
reviewed, merged, and backported on its own.
   
   ### Why are the changes needed?
   
   `var_pop`, `covar_pop`, `regr_sxy`, `corr`, and `regr_r2` can return `NaN` 
for finite inputs when a partial/final aggregate merge encounters an empty 
buffer against a large-magnitude average. The correct results are finite (e.g. 
zero variance for equal values). The bug is only masked when adjacent 
aggregates are combined into `Complete` mode (which skips the merge), so the 
same query returns different results depending on whether the merge path runs.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. Queries using `corr`, `covar_pop`/`covar_samp`, `var_pop`/`var_samp`, 
`stddev_*`, `skewness`, `kurtosis`, and the `regr_*` family that previously 
returned `NaN` for large finite inputs now return the correct finite result.
   
   ### How was this patch tested?
   
   Two new regression tests in `DataFrameAggregateSuite`:
   - `SPARK-58291`-covered `var_pop`/`covar_pop`/`regr_sxy` on two equal large 
values (expect `0.0`).
   - `corr`/`regr_r2` on two large-magnitude finite points (expect a finite 
result, not `NaN`).
   
   Both assert results are stable across AQE on/off and 
`combineAdjacentAggregation` on/off, and both fail without the fix on the merge 
path. `catalyst/compile` and the new tests pass.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Claude Opus 4.8)
   


-- 
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]

Reply via email to