Garamda commented on code in PR #13511:
URL: https://github.com/apache/datafusion/pull/13511#discussion_r1980826096
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -349,15 +365,49 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
} else {
// User defined aggregate functions (UDAF) have precedence in case
it has the same name as a scalar built-in function
if let Some(fm) = self.context_provider.get_aggregate_meta(&name) {
- let order_by = self.order_by_to_sort_expr(
- order_by,
- schema,
- planner_context,
- true,
- None,
- )?;
- let order_by = (!order_by.is_empty()).then_some(order_by);
- let args = self.function_args_to_expr(args, schema,
planner_context)?;
+ if fm.is_ordered_set_aggregate() && within_group.is_empty() {
+ return plan_err!("WITHIN GROUP clause is required when
calling ordered set aggregate function({})", fm.name());
+ }
+
+ if null_treatment.is_some() &&
!fm.supports_null_handling_clause() {
+ return plan_err!(
+ "[IGNORE | RESPECT] NULLS are not permitted for {}",
+ fm.name()
+ );
+ }
Review Comment:
> This was smelling odd, so I dug a bit deeper. I think you've inadvertantly
stumbled into something even weirder than you anticipated
> ...
> The RESPECT NULLS | IGNORE NULLS options is only a property of certain
window functions, hence we shouldn't need to track it for aggregate functions.
>
> I'm going to file a ticket for the above.
> ...
> Per my point about [IGNORE | RESPECT] NULLS being a property of window
functions, I don't think we need this check here.
I understood and agree with your guidance.
I will track what is decided in the issue you filed, and will remove some
codes out after determination.
--
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]
