duc12111 opened a new pull request, #21920:
URL: https://github.com/apache/datafusion/pull/21920
## Which issue does this PR close?
- Closes #21697.
## 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".
This is an oversight from PRs #17378 and #16925 which added
`filter`/`distinct` fields to `WindowFunctionParams` and updated `build()`, but
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.
- 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 these changes tested?
Yes. 4 new unit tests added in `datafusion/expr/src/expr_fn.rs`. All 200
tests in `datafusion-expr` pass. The `build()` method already correctly applies
`filter` and `distinct` to both `AggregateFunction` and `WindowFunction`
params, so no changes were needed there.
## Are there any user-facing changes?
No breaking changes. This is a bug fix that makes `ExprFunctionExt` method
chaining order-independent for `filter()` and `distinct()` on window function
expressions, matching the existing behavior of `order_by()` and
`null_treatment()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
