## 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 #https://github.com/apache/datafusion/issues/20024.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
## 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.
-->
- part of #https://github.com/apache/datafusion/issues/20024.
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
## Which issue does this PR close?
Closes#18109.
## Rationale for this change
Previously, the SQL planner accepted `WITHIN GROUP` clauses for all
aggregate UDAFs, even those that did not explicitly support ordered-set
semantics. This behavior was too permissive and inconsistent with
PostgreSQL. For example, queries such as `SUM(x) WITHIN GROUP (ORDER BY
x)` were allowed, even though `SUM` is not an ordered-set aggregate.
This PR enforces stricter validation so that only UDAFs that explicitly
return `true` from `supports_within_group_clause()` may use `WITHIN
GROUP`. All other aggregates now produce a clear planner error when this
syntax is used.
## What changes are included in this PR?
* Added type alias `WithinGroupExtraction` to simplify complex tuple
return types used by helper functions.
* Introduced a new helper method `extract_and_prepend_within_group_args`
to centralize logic for handling `WITHIN GROUP` argument rewriting.
* Updated the planner to:
* Validate that only UDAFs with `supports_within_group_clause()` can
accept `WITHIN GROUP`.
* Prepend `WITHIN GROUP` ordering expressions to function arguments only
for supported ordered-set aggregates.
* Produce clear error messages when `WITHIN GROUP` is used incorrectly.
* Added comprehensive unit tests verifying correct behavior and failure
cases:
* `WITHIN GROUP` rejected for non-ordered-set aggregates (`MIN`, `SUM`,
etc.).
* `WITHIN GROUP` accepted for ordered-set aggregates such as
`percentile_cont`.
* Validation for named arguments, multiple ordering expressions, and
semantic conflicts with `OVER` clauses.
* Updated SQL logic tests (`aggregate.slt`) to reflect new rejection
behavior.
* Updated documentation:
* `aggregate_functions.md` and developer docs to clarify when and how
`WITHIN GROUP` can be used.
* `upgrading.md` to inform users of this stricter enforcement and
migration guidance.
## Are these changes tested?
✅ Yes.
* New tests in `sql_integration.rs` validate acceptance, rejection, and
argument behavior of `WITHIN GROUP` for both valid and invalid cases.
* SQL logic tests (`aggregate.slt`) include negative test cases
confirming planner rejections.
## Are there any user-facing changes?
✅ Yes.
* Users attempting to use `WITHIN GROUP` with regular aggregates (e.g.
`SUM`, `AVG`, `MIN`, `MAX`) will now see a planner error:
> `WITHIN GROUP is only supported for ordered-set aggregate functions`
* Documentation has been updated to clearly describe `WITHIN GROUP`
semantics and provide examples of valid and invalid usage.
No API-breaking changes were introduced; only stricter planner
validation and improved error messaging.
* feat: Support `FILTER` clause in aggregate window functions
* fix: Box `WindowFunction` in `ExprFuncKind` enum to reduce enum total size
As suggested by `clippy`:
```
warning: large size difference between variants
--> datafusion/expr/src/expr_fn.rs:772:1
|
772 | / pub enum ExprFuncKind {
773 | | Aggregate(AggregateFunction),
| | ---------------------------- the second-largest variant contains at least 72 bytes
774 | | Window(WindowFunction),
| | ---------------------- the largest variant contains at least 288 bytes
775 | | }
| |_^ the entire enum is at least 288 bytes
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
= note: `#[warn(clippy::large_enum_variant)]` on by default
help: consider boxing the large fields to reduce the total size of the enum
|
774 - Window(WindowFunction),
774 + Window(Box<WindowFunction>),
|
```
* test: Add DataFrame API test for FILTER clause on aggregate window functions
* docs: Update aggregate and window function documentation with FILTER support
* docs: Link missing proto fields to github issue in TODO comment
* Initial work on #12432 to allow for generation of udf docs from embedded documentation in the code
* Add missing license header.
* Fixed examples.
* Fixing a really weird RustRover/wsl ... something. No clue what happened there.
* permission change
* Cargo fmt update.
* Refactored Documentation to allow it to be used in a const.
* Add documentation for syntax_example
* Refactoring Documentation based on PR feedback.
* Cargo fmt update.
* Doc update
* Fixed copy/paste error.
* Minor text updates.
---------
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sergey Zhukov