Skip to content

feat: fix AVG sliding windows wrong results with NULLs#22139

Open
comphead wants to merge 1 commit into
apache:mainfrom
comphead:windows_avg
Open

feat: fix AVG sliding windows wrong results with NULLs#22139
comphead wants to merge 1 commit into
apache:mainfrom
comphead:windows_avg

Conversation

@comphead
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

AVG used as a window aggregate can return NaN (and, for Decimal / Duration, panic on integer division by zero) when every value in the window frame is NULL.

SELECT i,
       AVG(v) OVER (ORDER BY i ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)
FROM (VALUES (1,1), (2,2), (3,NULL), (4,NULL)) t(i,v);
i current output expected (DuckDB/PgSQL)
1 1.5 1.5
2 2.0 2.0
3 NaN NULL
4 NaN NULL

Root cause: sliding-window execution calls Accumulator::retract_batch as rows leave the frame. Once every contributing value has been retracted, self.count drops back to 0 but self.sum stays
Some(0.0) (or a tiny floating-point residual). evaluate() then computes sum / 0, which yields NaN on Float64, and would panic with integer division by zero on DecimalAvgAccumulator and
DurationAvgAccumulator.

The non-sliding aggregation path is unaffected because there sum becomes Some(_) only after at least one non-NULL value has been added, so count == 0 implies sum == None.

What changes are included in this PR?

datafusion/functions-aggregate/src/average.rs — guard all three affected evaluate() implementations with an explicit count == 0 → None short-circuit:

  • AvgAccumulator::evaluate (Float64)
  • DecimalAvgAccumulator::evaluate (Decimal32/64/128/256)
  • DurationAvgAccumulator::evaluate (Duration*)

This matches the idiom already used by sibling retractable accumulators (variance.rs uses an explicit match self.count before division; sum.rs uses a (self.count != 0).then_some(..) guard).

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels May 13, 2026
Copy link
Copy Markdown
Contributor

@2010YOUY01 2010YOUY01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@alamb alamb added this pull request to the merge queue May 13, 2026
@comphead comphead changed the title feat: fix AVG sliding windows edge case feat: fix AVG sliding windows wrong results with NULLs May 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows AVG wrong result for NULLs

2 participants