You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/05/13 16:57:11 UTC
(datafusion) branch gh-readonly-queue/main/pr-22140-0c4ace8b77461cac3538d38b1122a7dbf08c4d4f created (now 7b33632029)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-22140-0c4ace8b77461cac3538d38b1122a7dbf08c4d4f
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 7b33632029 feat: fix windows frame positive/neg overflows (#22140)
This branch includes the following new commits:
new 7b33632029 feat: fix windows frame positive/neg overflows (#22140)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: feat: fix windows frame positive/neg overflows (#22140)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-22140-0c4ace8b77461cac3538d38b1122a7dbf08c4d4f
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 7b3363202938ea6ab020268ece0321d79bac8c1c
Author: Oleks V <co...@users.noreply.github.com>
AuthorDate: Wed May 13 09:56:49 2026 -0700
feat: fix windows frame positive/neg overflows (#22140)
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes #123` indicates that this PR will close issue #123.
-->
- Closes #22137 .
## 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.
---
datafusion/expr/src/window_state.rs | 57 ++++---
datafusion/sqllogictest/test_files/window.slt | 220 ++++++++++++++++++++++++++
2 files changed, 254 insertions(+), 23 deletions(-)
diff --git a/datafusion/expr/src/window_state.rs b/datafusion/expr/src/window_state.rs
index d7da7a778b..f8d4609d36 100644
--- a/datafusion/expr/src/window_state.rs
+++ b/datafusion/expr/src/window_state.rs
@@ -396,6 +396,11 @@ impl WindowFrameStateRange {
length: usize,
) -> Result<usize> {
let current_row_values = get_row_at_idx(range_columns, idx)?;
+ let search_start = if SIDE {
+ last_range.start
+ } else {
+ last_range.end
+ };
let end_range = if let Some(delta) = delta {
let is_descending: bool = self
.sort_options
@@ -407,34 +412,40 @@ impl WindowFrameStateRange {
})?
.descending;
- current_row_values
- .iter()
- .map(|value| {
- if value.is_null() {
- return Ok(value.clone());
+ // On overflow the boundary exceeds the type's range and is
+ // effectively unbounded within the partition. Collapse to the
+ // partition edge rather than feeding `search_in_slice` a
+ // wrapped-around target: PRECEDING searches reach `search_start`,
+ // FOLLOWING searches reach `length`.
+ let unbounded_edge = if SEARCH_SIDE { search_start } else { length };
+ let mut targets = Vec::with_capacity(current_row_values.len());
+ for value in ¤t_row_values {
+ if value.is_null() {
+ targets.push(value.clone());
+ continue;
+ }
+ let target = if SEARCH_SIDE == is_descending {
+ match value.add_checked(delta) {
+ Ok(v) => v,
+ Err(_) => return Ok(unbounded_edge),
}
- if SEARCH_SIDE == is_descending {
- // TODO: Handle positive overflows.
- value.add(delta)
- } else if value.is_unsigned() && value < delta {
- // NOTE: This gets a polymorphic zero without having long coercion code for ScalarValue.
- // If we decide to implement a "default" construction mechanism for ScalarValue,
- // change the following statement to use that.
- value.sub(value)
- } else {
- // TODO: Handle negative overflows.
- value.sub(delta)
+ } else if value.is_unsigned() && value < delta {
+ // NOTE: This gets a polymorphic zero without having long coercion code for ScalarValue.
+ // If we decide to implement a "default" construction mechanism for ScalarValue,
+ // change the following statement to use that.
+ value.sub(value)?
+ } else {
+ match value.sub_checked(delta) {
+ Ok(v) => v,
+ Err(_) => return Ok(unbounded_edge),
}
- })
- .collect::<Result<Vec<ScalarValue>>>()?
+ };
+ targets.push(target);
+ }
+ targets
} else {
current_row_values
};
- let search_start = if SIDE {
- last_range.start
- } else {
- last_range.end
- };
let compare_fn = |current: &[ScalarValue], target: &[ScalarValue]| {
let cmp = compare_rows(current, target, &self.sort_options)?;
Ok(if SIDE { cmp.is_lt() } else { cmp.is_le() })
diff --git a/datafusion/sqllogictest/test_files/window.slt b/datafusion/sqllogictest/test_files/window.slt
index 74c2e38baa..2a74660fe9 100644
--- a/datafusion/sqllogictest/test_files/window.slt
+++ b/datafusion/sqllogictest/test_files/window.slt
@@ -6236,6 +6236,226 @@ INNER JOIN issue_20194_t2 t2
----
6774502793 10040029 1
+# Regression tests for RANGE window frames whose value-offset boundary
+# computation overflows the type's representable range. Previously these
+# queries panicked in functions-window/src/nth_value.rs with
+# "attempt to subtract with overflow" because the wrapped-around target
+# produced a frame range where `end < start`. Both positive overflows
+# (target above type MAX) and negative overflows (target below type MIN)
+# must be treated as unbounded within the partition.
+
+############################################################################
+# Positive overflow: value + delta exceeds type MAX
+############################################################################
+
+# ASC + FOLLOWING: end bound wraps past i64::MAX.
+query II
+SELECT a, last_value(a) OVER (ORDER BY a RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT 9223372036854775804 AS a
+ UNION ALL SELECT 9223372036854775805
+ UNION ALL SELECT 9223372036854775806
+);
+----
+9223372036854775804 9223372036854775806
+9223372036854775805 9223372036854775806
+9223372036854775806 9223372036854775806
+
+query II
+SELECT a, first_value(a) OVER (ORDER BY a RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT 9223372036854775804 AS a
+ UNION ALL SELECT 9223372036854775805
+ UNION ALL SELECT 9223372036854775806
+);
+----
+9223372036854775804 9223372036854775804
+9223372036854775805 9223372036854775805
+9223372036854775806 9223372036854775806
+
+# Symmetric PRECEDING/FOLLOWING where the FOLLOWING side overflows past MAX.
+query II
+SELECT a, last_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND 4 FOLLOWING)
+FROM (
+ SELECT 9223372036854775804 AS a
+ UNION ALL SELECT 9223372036854775805
+ UNION ALL SELECT 9223372036854775806
+);
+----
+9223372036854775804 9223372036854775806
+9223372036854775805 9223372036854775806
+9223372036854775806 9223372036854775806
+
+# DESC + PRECEDING: "PRECEDING" walks toward larger values in DESC order,
+# so offsetting past i64::MAX exercises the ADD-overflow path.
+query II
+SELECT a, first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT 9223372036854775804 AS a
+ UNION ALL SELECT 9223372036854775805
+ UNION ALL SELECT 9223372036854775806
+);
+----
+9223372036854775806 9223372036854775806
+9223372036854775805 9223372036854775806
+9223372036854775804 9223372036854775806
+
+query II
+SELECT a, last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT 9223372036854775804 AS a
+ UNION ALL SELECT 9223372036854775805
+ UNION ALL SELECT 9223372036854775806
+);
+----
+9223372036854775806 9223372036854775806
+9223372036854775805 9223372036854775805
+9223372036854775804 9223372036854775804
+
+# Unsigned ordering column: add past u64::MAX must not wrap.
+query II
+SELECT a, last_value(a) OVER (ORDER BY a RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT arrow_cast(18446744073709551612, 'UInt64') AS a
+ UNION ALL SELECT arrow_cast(18446744073709551613, 'UInt64')
+ UNION ALL SELECT arrow_cast(18446744073709551614, 'UInt64')
+);
+----
+18446744073709551612 18446744073709551614
+18446744073709551613 18446744073709551614
+18446744073709551614 18446744073709551614
+
+query II
+SELECT a, first_value(a) OVER (ORDER BY a RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT arrow_cast(18446744073709551612, 'UInt64') AS a
+ UNION ALL SELECT arrow_cast(18446744073709551613, 'UInt64')
+ UNION ALL SELECT arrow_cast(18446744073709551614, 'UInt64')
+);
+----
+18446744073709551612 18446744073709551612
+18446744073709551613 18446744073709551613
+18446744073709551614 18446744073709551614
+
+############################################################################
+# Negative overflow: value - delta falls below type MIN
+############################################################################
+
+# ASC + PRECEDING: start bound wraps below i64::MIN.
+query II
+SELECT a, first_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT -9223372036854775807 AS a
+ UNION ALL SELECT -9223372036854775806
+ UNION ALL SELECT -9223372036854775805
+);
+----
+-9223372036854775807 -9223372036854775807
+-9223372036854775806 -9223372036854775807
+-9223372036854775805 -9223372036854775807
+
+query II
+SELECT a, last_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT -9223372036854775807 AS a
+ UNION ALL SELECT -9223372036854775806
+ UNION ALL SELECT -9223372036854775805
+);
+----
+-9223372036854775807 -9223372036854775807
+-9223372036854775806 -9223372036854775806
+-9223372036854775805 -9223372036854775805
+
+# Symmetric PRECEDING/FOLLOWING where the PRECEDING side underflows past MIN.
+query II
+SELECT a, first_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND 4 FOLLOWING)
+FROM (
+ SELECT -9223372036854775807 AS a
+ UNION ALL SELECT -9223372036854775806
+ UNION ALL SELECT -9223372036854775805
+);
+----
+-9223372036854775807 -9223372036854775807
+-9223372036854775806 -9223372036854775807
+-9223372036854775805 -9223372036854775807
+
+# DESC + FOLLOWING: "FOLLOWING" walks toward smaller values in DESC order,
+# so offsetting past i64::MIN exercises the SUB-underflow path.
+query II
+SELECT a, last_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT -9223372036854775805 AS a
+ UNION ALL SELECT -9223372036854775806
+ UNION ALL SELECT -9223372036854775807
+);
+----
+-9223372036854775805 -9223372036854775807
+-9223372036854775806 -9223372036854775807
+-9223372036854775807 -9223372036854775807
+
+query II
+SELECT a, first_value(a) OVER (ORDER BY a DESC RANGE BETWEEN CURRENT ROW AND 4 FOLLOWING)
+FROM (
+ SELECT -9223372036854775805 AS a
+ UNION ALL SELECT -9223372036854775806
+ UNION ALL SELECT -9223372036854775807
+);
+----
+-9223372036854775805 -9223372036854775805
+-9223372036854775806 -9223372036854775806
+-9223372036854775807 -9223372036854775807
+
+# Unsigned ordering column: subtracting an offset that would go below 0
+# must saturate to 0, not wrap to u64::MAX.
+query II
+SELECT a, first_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT arrow_cast(1, 'UInt64') AS a
+ UNION ALL SELECT arrow_cast(2, 'UInt64')
+ UNION ALL SELECT arrow_cast(3, 'UInt64')
+);
+----
+1 1
+2 1
+3 1
+
+query II
+SELECT a, last_value(a) OVER (ORDER BY a RANGE BETWEEN 4 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT arrow_cast(1, 'UInt64') AS a
+ UNION ALL SELECT arrow_cast(2, 'UInt64')
+ UNION ALL SELECT arrow_cast(3, 'UInt64')
+);
+----
+1 1
+2 2
+3 3
+
+############################################################################
+# ROWS frame regression guard: huge offsets already saturate via
+# saturating_sub / min(length), verify we keep that behavior.
+############################################################################
+
+query II
+SELECT a, last_value(a) OVER (ORDER BY a ROWS BETWEEN CURRENT ROW AND 9223372036854775807 FOLLOWING)
+FROM (
+ SELECT 1 AS a UNION ALL SELECT 2 UNION ALL SELECT 3
+);
+----
+1 3
+2 3
+3 3
+
+query II
+SELECT a, first_value(a) OVER (ORDER BY a ROWS BETWEEN 9223372036854775807 PRECEDING AND CURRENT ROW)
+FROM (
+ SELECT 1 AS a UNION ALL SELECT 2 UNION ALL SELECT 3
+);
+----
+1 1
+2 1
+3 1
+
# Config reset
statement ok
reset datafusion.execution.batch_size;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org