Skip to content

feat: fix windows frame positive/neg overflows#22140

Merged
comphead merged 3 commits into
apache:mainfrom
comphead:windows_lv
May 13, 2026
Merged

feat: fix windows frame positive/neg overflows#22140
comphead merged 3 commits into
apache:mainfrom
comphead:windows_lv

Conversation

@comphead
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

RANGE window frames with a value offset (e.g. RANGE BETWEEN 4 PRECEDING AND 4 FOLLOWING) panicked with attempt to add/subtract with overflow whenever the boundary target (value ± delta) wrapped
past the type's representable range. Affected inputs include values close to i64::MAX/i64::MIN, u64::MAX, and any analogous boundary for other integer/decimal/timestamp types.

Two // TODO: Handle ... overflows. markers in WindowFrameStateRange::calculate_index_of_row had been left for this case; the unchecked ScalarValue::add / sub silently wrapped the target, after
which search_in_slice was handed a nonsensical (wrapped) value and downstream code tripped a debug-assert subtraction in functions-window/src/nth_value.rs.

Semantically, an overflowed boundary is unbounded with respect to the data in the partition — every real value lies strictly inside the wrapped sentinel — so the correct behavior is to collapse the
search to the appropriate partition edge rather than to search with a wrapped target.

What changes are included in this PR?

datafusion/expr/src/window_state.rs

  • Replace ScalarValue::add / sub with their *_checked counterparts in the boundary computation.
  • On overflow, short-circuit to the correct partition edge: search_start for PRECEDING-direction searches, length for FOLLOWING-direction searches. The collapse direction depends only on the
    const-generic SEARCH_SIDE (the add branch and sub branch both reduce to !SEARCH_SIDE once you expand the SEARCH_SIDE == is_descending invariant that selects each arithmetic branch).
  • The pre-existing value.is_unsigned() && value < delta clamp-to-zero path for unsigned subtraction is preserved — it produces a valid polymorphic zero, not an overflow sentinel.
  • No behavior change on the non-overflow path.

datafusion/sqllogictest/test_files/window.slt

Regression coverage for positive and negative overflow, across:

  • ASC + FOLLOWING / ASC + PRECEDING / DESC + PRECEDING / DESC + FOLLOWING (each overflow direction occurs on both sort orders depending on which arithmetic branch is taken)
  • Symmetric N PRECEDING AND N FOLLOWING frames where only one side overflows
  • Signed (i64) and unsigned (u64) ordering columns
  • first_value and last_value both exercised to verify both frame edges
  • ROWS frame regression guard to document that the pre-existing saturating_sub / min(length) saturation behavior is unchanged.

@github-actions github-actions Bot added logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels May 13, 2026
Copy link
Copy Markdown
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

reasonable fix, thank you1

Err(_) => return Ok(unbounded_edge),
}
if SEARCH_SIDE == is_descending {
// TODO: Handle positive overflows.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 task completed

@comphead comphead added this pull request to the merge queue May 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch May 13, 2026
@comphead comphead added this pull request to the merge queue May 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 13, 2026
@comphead comphead added this pull request to the merge queue May 13, 2026
@comphead comphead removed this pull request from the merge queue due to a manual request May 13, 2026
@comphead comphead added this pull request to the merge queue May 13, 2026
@comphead comphead removed this pull request from the merge queue due to a manual request May 13, 2026
@comphead comphead added this pull request to the merge queue May 13, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch May 13, 2026
@comphead comphead enabled auto-merge May 13, 2026 22:10
@comphead comphead disabled auto-merge May 13, 2026 22:10
@comphead comphead enabled auto-merge May 13, 2026 22:10
@comphead comphead added this pull request to the merge queue May 13, 2026
Merged via the queue into apache:main with commit ccc67e9 May 13, 2026
38 checks passed
@comphead comphead deleted the windows_lv branch May 13, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows RANGE frame crashes on overflows

2 participants