kosiew commented on code in PR #18607:
URL: https://github.com/apache/datafusion/pull/18607#discussion_r2517577992
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -490,31 +496,35 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
let (mut args, mut arg_names) =
self.function_args_to_expr_with_names(args, schema,
planner_context)?;
- let order_by = if fm.supports_within_group_clause() {
- let within_group = self.order_by_to_sort_expr(
- within_group,
- schema,
- planner_context,
- false,
- None,
- )?;
-
- // Add the WITHIN GROUP ordering expressions to the front
of the argument list
- // So function(arg) WITHIN GROUP (ORDER BY x) becomes
function(x, arg)
- if !within_group.is_empty() {
- // Prepend None arg names for each WITHIN GROUP
expression
- let within_group_count = within_group.len();
- arg_names = std::iter::repeat_n(None,
within_group_count)
- .chain(arg_names)
- .collect();
-
- args = within_group
- .iter()
- .map(|sort| sort.expr.clone())
- .chain(args)
- .collect::<Vec<_>>();
- }
- within_group
+ // Enforce explicit opt-in for WITHIN GROUP: UDAFs must
advertise
+ // `supports_within_group_clause()` to accept WITHIN GROUP
syntax.
+ // Previously the planner forwarded WITHIN GROUP to any
+ // order-sensitive aggregate; that was too permissive.
+ let supports_within_group = fm.supports_within_group_clause();
+
+ if !within_group.is_empty() && !supports_within_group {
+ return plan_err!(
+ "WITHIN GROUP is only supported for ordered-set
aggregate functions"
+ );
+ }
+
+ // If the UDAF opted into WITHIN GROUP handling, convert the
+ // WITHIN GROUP ordering into sort expressions and prepend the
+ // ordering expressions to the function argument list (as
unnamed
+ // args). This helper centralizes the argument-prepending
protocol
+ // so it's easier to maintain and reason about.
Review Comment:
Amended to be more succinct.
--
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]
