neilconway commented on code in PR #23813:
URL: https://github.com/apache/datafusion/pull/23813#discussion_r3633134896
##########
datafusion/expr/src/utils.rs:
##########
@@ -652,6 +652,88 @@ pub fn find_aggregate_exprs<'a>(exprs: impl
IntoIterator<Item = &'a Expr>) -> Ve
})
}
+/// Returns an error if any of `exprs` contains an aggregate function call
+/// nested inside another aggregate function call, such as `sum(sum(x))`.
+///
+/// Such expressions are not valid SQL (PostgreSQL reports `aggregate function
+/// calls cannot be nested`) and have no physical equivalent, so they are
+/// rejected while the logical plan is built rather than failing later with an
+/// error that does not point back at the original SQL.
+///
+/// The nested aggregate is searched for in the arguments, `FILTER` and
+/// `ORDER BY` of each aggregate function call. Note that an aggregate used as
+/// the argument of a *window* function, such as `sum(sum(x)) OVER ()`, is
+/// legal and accepted: there the inner aggregate is evaluated by the
+/// `Aggregate` node and the window function is evaluated on top of its result.
+pub fn check_no_nested_aggregates<'a>(
+ exprs: impl IntoIterator<Item = &'a Expr>,
+) -> Result<()> {
+ for expr in exprs {
+ expr.apply(|outer| {
+ if !matches!(outer, Expr::AggregateFunction(_)) {
+ return Ok(TreeNodeRecursion::Continue);
+ }
+
+ // `outer` is an aggregate, so no other aggregate may appear
+ // anywhere below it.
+ let mut nested: Option<&Expr> = None;
+ outer.apply_children(|child| {
+ child.apply(|inner| {
+ if matches!(inner, Expr::AggregateFunction(_)) {
+ nested = Some(inner);
+ Ok(TreeNodeRecursion::Stop)
+ } else {
+ Ok(TreeNodeRecursion::Continue)
+ }
+ })
+ })?;
+
+ match nested {
+ Some(inner) => nested_aggregate_err(outer, inner),
+ // The whole subtree below `outer` has just been checked
+ None => Ok(TreeNodeRecursion::Jump),
+ }
+ })?;
+ }
+ Ok(())
+}
+
+fn nested_aggregate_err(outer: &Expr, inner: &Expr) ->
Result<TreeNodeRecursion> {
+ let diagnostic = Diagnostic::new_error(
+ "aggregate function calls cannot be nested",
+ first_span(inner),
+ )
+ .with_help(
+ format!(
+ "Compute '{inner}' in an inner query and aggregate its result, or
use a window function such as '{outer} OVER ()'"
+ ),
Review Comment:
Seems like this suggests illegal syntax if the problematic construct occurs
in a `FILTER` or `ORDER BY` clause.
--
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]
