IgnatiusPang opened a new pull request, #25743:
URL: https://github.com/apache/datafusion/pull/25743
## Rationale for this change
In `datafusion-functions-aggregate::variance`:
There is a semantic inconsistency between the scalar accumulator path
(`VarianceAccumulator::evaluate`, used without `GROUP BY`) and the grouped
accumulator path (`VarianceGroupsAccumulator::variance_values`, used with
`GROUP BY`) on single-row datasets containing `NaN`:
1. **The Inconsistency**:
- `VarianceGroupsAccumulator` evaluates `m2 / count` on non-zero groups.
For `StatsType::Population` with `count == 1` and `val = NaN`, `m2` is `NaN`,
so `m2 / 1.0` produces `NaN`.
- `VarianceAccumulator` hardcoded `match self.count { 1 => if
self.stats_type == StatsType::Population { Some(0.0) } ... }`, completely
ignoring whether `self.m2` was `NaN`.
- `DistinctVarianceAccumulator` similarly hardcoded `Some(0.0)` on
`values.len() == 1`.
2. **User-visible symptom**:
- `SELECT var_pop(val) FROM t` returned `0.0` for a single-row dataset
with `val = NaN`.
- `SELECT group_col, var_pop(val) FROM t GROUP BY group_col` returned
`NaN`.
The exact same input yielded different results depending on whether
`GROUP BY` was present.
## What changes are included in this PR?
1. **`VarianceAccumulator::evaluate`**:
- Check `if self.m2.is_nan() { Some(f64::NAN) }` before returning
`Some(0.0)` for single-row population variance.
2. **`DistinctVarianceAccumulator::evaluate`**:
- Check `if m2.is_nan() { Some(f64::NAN) }` before returning `Some(0.0)`.
3. **Formal Verification (Lean 4)**:
- Formally verified theorem contract synthesized and applied via Lean 4
Three-Way Semantic Merge:
- `variance_accumulator_nan_propagation`
4. **Regression Tests**:
- Added `test_variance_nan_single_row_scalar_and_grouped_consistency`
verifying that `var_pop(NaN)` evaluates to `NaN` consistently across scalar,
group, and distinct accumulators.
## What is the testing strategy for this PR?
- Full crate test suite pass (244 tests total):
```bash
cargo test -p datafusion-functions-aggregate
--
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]
