kita-renji opened a new pull request, #25860: URL: https://github.com/apache/datafusion/pull/25860
## Which issue does this PR close? - Closes #25859. ## Rationale for this change Window aggregates over a `RANGE ... N PRECEDING` frame return results that depend on how the input is split into batches when the ORDER BY value is NULL. For a NULL row the frame is its group of NULL peers, and when that group spans batches the row is computed before the rest of the group arrives. At default settings a query over 20,000 rows with 10,000 NULL keys returns a count of 6384 instead of 10000 for 6,384 of the NULL rows. Details and repros are in the issue. ## What changes are included in this PR? - `WindowFrame::new_bounds`: a `RANGE` frame is causal only when it ends at `UNBOUNDED PRECEDING`. With an offset end bound, the frame of a NULL or NaN row ends at its last peer, which can be a later row. `GROUPS` keeps the current rule, since NULL peers form an ordinary group there. - `is_end_bound_safe_for_range`: a `PRECEDING` end bound is handled like `CURRENT ROW`, so the row waits until an input row past its peer group arrives (`is_row_ahead`) or the partition ends. An `N PRECEDING` frame only reaches the end of the buffer when the row's frame ends at its last peer, so the check only holds back NULL and NaN rows. Rows with other ORDER BY values behave as before: their `N PRECEDING` frame ends before the current row, so it never reaches the end of the buffer and results are still produced as the input streams. RANGE frames planned from SQL were already non-causal (their bounds are strings until type coercion), so their plans don't change; the second change is what fixes them. The first change fixes frames built from typed bounds (reversed frames, the DataFrame API, proto), where built-in window functions such as `nth_value` were affected too. ## What is the testing strategy for this PR? New cases at the end of `window.slt`, with `batch_size = 2` so the peers span batches: `sum`/`count`/`max` over `5 PRECEDING AND 1 PRECEDING` and `UNBOUNDED PRECEDING AND 1 PRECEDING`, NULLS FIRST and DESC orderings, a `FOLLOWING` frame that the planner reverses to reuse another window's sort (covering `nth_value`), and a group of NaN keys. Expected values match DuckDB 1.5.5 and Postgres 17, and all four queries fail without the fix. I also ran a randomized check outside the test suite: random tables with NULL keys, random RANGE/GROUPS/ROWS frames and functions whose result doesn't depend on tie order, compared across batch sizes 1, 2, 3, 7 and 8192 and against DuckDB. Main had 31 batch-dependent results in the first 1,000 queries; this branch had none in 7,500, and none in variants with NaN keys and with an unbounded ordered source (Linear mode, PARTITION BY). The whole sqllogictest suite and the window fuzz tests (`--features extended_tests`) pass. ## Are there any user-facing changes? Queries with NULL or NaN values in the ORDER BY column of a `RANGE ... N PRECEDING` window now return the same results regardless of batch size. `WindowFrame::is_causal()` now returns false for RANGE frames with an offset end bound. For frames built from typed bounds this can drop an ordering that a set-monotonic aggregate used to add to the window output, so a plan may need an extra sort; SQL-planned frames were already non-causal, and no sqllogictest plan changed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
