Skip to content

Commit fc9f146

Browse files
authored
fix: validate trailing ORDER BY keys in free RANGE frames (#25518)
## Which issue does this PR close? - Closes #25557. ## Rationale for this change A free `RANGE` window frame determines peers using every `ORDER BY` expression. However, when the first expression had an offset target type, the analyzer did not validate the remaining expressions. Consequently, an unsupported trailing `Map` expression could reach execution and rows with different map values could incorrectly be treated as peers. ## What changes are included in this PR? Validate all trailing `ORDER BY` expressions when the first expression establishes the RANGE target type. This preserves the existing behavior that accepts a `List` as the first `ORDER BY` expression, while rejecting unsupported trailing expressions. A SQL logic regression test for `ORDER BY int_col, map_col` is included. ## What is the testing strategy for this PR? - Added a regression test in `window.slt`. - Verified that the old code returns incorrect peer counts while the fixed code reports a planning error. - Verified existing first-`List` and supported multi-key cases. - Ran `cargo fmt --all -- --check`. - Ran `cargo clippy -p datafusion-optimizer --all-targets --all-features -- -D warnings`. ## Are there any user-facing changes? Yes. Free `RANGE` frames with an unsupported trailing `ORDER BY` type are now rejected during planning instead of potentially producing incorrect results. There are no public API changes.
1 parent 19d422f commit fc9f146

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

‎datafusion/optimizer/src/analyzer/type_coercion.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,14 @@ fn coerce_window_frame(
12001200
.transpose()?;
12011201
if let Some(col_type) = current_types {
12021202
let target_type = match extract_window_frame_target_type(&col_type) {
1203-
Some(target_type) => target_type,
1203+
Some(target_type) => {
1204+
if window_frame.free_range() {
1205+
// The first key established the target type above, but
1206+
// every later key also participates in peer comparison.
1207+
check_free_range_order_by_types(&expressions[1..], schema)?;
1208+
}
1209+
target_type
1210+
}
12041211
// A free range frame has no offsets to coerce, so ORDER BY
12051212
// types without arithmetic are fine as long as their peer
12061213
// comparison is sound (see `supports_free_range_frame`).

‎datafusion/sqllogictest/test_files/window.slt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7129,6 +7129,12 @@ select count(*) over (order by x) from (values (map(['a'], [1])), (map(['a'], [2
71297129
statement error Error during planning: RANGE window frames are not supported for ORDER BY type Map
71307130
select count(*) over (order by d, m) from (values (arrow_cast(1, 'Duration(Second)'), map(['a'], [1])), (arrow_cast(1, 'Duration(Second)'), map(['a'], [2]))) t(d, m)
71317131

7132+
# the trailing key check also runs when the first ORDER BY type supports offset
7133+
# arithmetic; otherwise these maps are treated as peers because their keys are
7134+
# equal even though their values differ
7135+
statement error Error during planning: RANGE window frames are not supported for ORDER BY type Map
7136+
select count(*) over (order by i, m) from (values (1, map(['a'], [1])), (1, map(['a'], [2]))) t(i, m)
7137+
71327138
query ??I
71337139
select d, e, count(*) over (order by d, e) from (values (arrow_cast(1, 'Duration(Second)'), arrow_cast(2, 'Duration(Millisecond)')), (arrow_cast(1, 'Duration(Second)'), arrow_cast(1, 'Duration(Millisecond)'))) t(d, e) order by 3
71347140
----

0 commit comments

Comments
 (0)