Skip to content

Commit 1e3b956

Browse files
buraksennjonahgao
andauthored
Minor: compute qualify window expressions only when QUALIFY clause is present (#21173)
## Which issue does this PR close? Does not close but related #608 ## Rationale for this change Followup to #20963 @jonahgao had a feedback in #20963 (comment) to try to find qualify only if we are inside QUALIFY block. I did not think it was this minor so thought followup would be better but this is simply moving to if block to prevent unnecessary computation ## What changes are included in this PR? Move `qualify_window_func_exprs` computation inside the QUALIFY block ## Are these changes tested? Covered by existing tests from #20963. ## Are there any user-facing changes? No Co-authored-by: Jonah Gao <jonahgao@msn.com>
1 parent 20434b0 commit 1e3b956

1 file changed

Lines changed: 5 additions & 8 deletions

File tree

datafusion/sql/src/select.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -293,14 +293,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
293293
plan
294294
};
295295

296-
// The window expressions from SELECT and QUALIFY only, used to validate that
297-
// QUALIFY is used with window functions (ORDER BY window functions don't count).
298-
let qualify_window_func_exprs = find_window_exprs(
299-
select_exprs_post_aggr
300-
.iter()
301-
.chain(qualify_expr_post_aggr.iter()),
302-
);
303-
304296
// All of the window expressions (deduplicated and rewritten to reference aggregates as
305297
// columns from input). Window functions may be sourced from the SELECT list, QUALIFY
306298
// expression, or ORDER BY.
@@ -342,6 +334,11 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
342334
// QUALIFY filters the results of window functions, similar to how HAVING filters aggregates
343335
let plan = if let Some(qualify_expr) = qualify_expr_post_aggr {
344336
// Validate that QUALIFY is used with window functions in SELECT or QUALIFY
337+
let qualify_window_func_exprs = find_window_exprs(
338+
select_exprs_post_aggr
339+
.iter()
340+
.chain(std::iter::once(&qualify_expr)),
341+
);
345342
if qualify_window_func_exprs.is_empty() {
346343
return plan_err!(
347344
"QUALIFY clause requires window functions in the SELECT list or QUALIFY clause"

0 commit comments

Comments
 (0)