ump45nose opened a new pull request, #26662: URL: https://github.com/apache/datafusion/pull/26662
## Which issue does this PR close? - Closes #26658. ## Rationale for this change A `ROWS` window frame with a huge `FOLLOWING` bound (for example `18446744073709551615 FOLLOWING`) combined with `ORDER BY ... LIMIT` panics with `attempt to add with overflow` in debug builds. In release builds the addition wraps, so `LimitPushPastWindows` could give the `SortExec` below the window a fetch that is far too small and drop rows the window needs. After fixing the optimizer rule, the same query panicked a second time in `WindowFrameContext::calculate_range_rows` (`window_state.rs`), which computes the frame end as `idx + n + 1` with the same unchecked bound. So both places need fixing for the query in the issue to return its result. ## What changes are included in this PR? - `limit_pushdown_past_window.rs`: use `saturating_add` when growing the limit by the window lookahead (`fetch + lookahead`, `lookahead + max_rel`) and when adding `skip` to a `GlobalLimitExec` fetch. A saturated fetch is still correct, it just means the sort keeps all rows. This matches how `input_fetch` in `sort_pushdown.rs` already handles the same kind of sum. - `window_state.rs`: use `saturating_add` for the `FOLLOWING n` start and end of a `ROWS` frame. The result is already clamped to the partition length, so saturating gives the right range. This touches the same lines in `limit_pushdown_past_window.rs` as the refactor in #26660, which leaves these sums to #26658. The change here is small, so whichever lands second should be easy to rebase. ## What is the testing strategy for this PR? - New cases in `window_limits.slt`: the query from the issue (`sum(x) OVER (... ROWS BETWEEN CURRENT ROW AND 18446744073709551615 FOLLOWING) ... LIMIT 2`), and a `lead(x, 1)` variant that adds its own lookahead on top of the huge frame bound. Without this change the first case panics at `limit_pushdown_past_window.rs:198`; with only the optimizer fix it panics at `window_state.rs:287`. - Locally: all `window*.slt` sqllogictests, `cargo test -p datafusion-expr --lib window_state`, `cargo test -p datafusion-physical-optimizer --lib limit_pushdown_past_window`, `cargo fmt --all` and `cargo clippy -p datafusion-expr -p datafusion-physical-optimizer --all-targets --all-features -- -D warnings` pass. ## Are there any user-facing changes? No API changes. Queries with very large `ROWS ... FOLLOWING` bounds now return results instead of panicking (or, in release builds, possibly returning too few rows). --- This PR was prepared with AI assistance. I reviewed the change and ran the tests listed above. -- 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]
