Skip to content

fix: handle WindowFunction in ExprFunctionExt filter and distinct on Expr - #21920

Closed
duc12111 wants to merge 1 commit into
apache:mainfrom
duc12111:fix-expr-function-ext-filter-distinct-window
Closed

fix: handle WindowFunction in ExprFunctionExt filter and distinct on Expr#21920
duc12111 wants to merge 1 commit into
apache:mainfrom
duc12111:fix-expr-function-ext-filter-distinct-window

Conversation

@duc12111

@duc12111 duc12111 commented Apr 29, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

In impl ExprFunctionExt for Expr, the filter() and distinct() methods only handle Expr::AggregateFunction. When called as the first method on an Expr::WindowFunction, they create an empty builder (fun: None), losing the expression entirely. .build() then errors with "ExprFunctionExt can only be used with Expr::AggregateFunction or Expr::WindowFunction".

After adding filter/distinct fields to WindowFunctionParams and updated build(), but ot did not update the Expr dispatch. The order_by() and null_treatment() methods already handle both variants correctly.

What changes are included in this PR?

  • Updated filter() in impl ExprFunctionExt for Expr to handle both AggregateFunction and WindowFunction variants, following the same pattern used by order_by() and null_treatment().
  • Updated distinct() in impl ExprFunctionExt for Expr with the same fix.

Are these changes tested?

Yes

Added 4 unit tests:

  • test_window_filter_first — verifies filter() works as first call on a WindowFunction
  • test_window_distinct_first — verifies distinct() works as first call on a WindowFunction
  • test_window_filter_then_partition_by — verifies chaining filter() then partition_by() on a WindowFunction
  • test_aggregate_filter_still_works — regression test confirming aggregates still work

Are there any user-facing changes?

No breaking changes.

@github-actions github-actions Bot added the logical-expr Logical plan and expressions label Apr 29, 2026
@duc12111
duc12111 force-pushed the fix-expr-function-ext-filter-distinct-window branch from 58e21be to 32d580a Compare April 29, 2026 13:46
@github-actions github-actions Bot added the core Core DataFusion crate label Apr 29, 2026
@duc12111
duc12111 force-pushed the fix-expr-function-ext-filter-distinct-window branch from 32d580a to 4e40499 Compare April 29, 2026 14:24
@github-actions

Copy link
Copy Markdown

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions Bot added the Stale PR has not had any activity for some time label Jun 29, 2026
@github-actions github-actions Bot closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate logical-expr Logical plan and expressions Stale PR has not had any activity for some time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

partition_by and filter on ExprFunctionExt

1 participant