fix(sql): handle GROUP BY ALL with aliased aggregates

datafusion/core/tests/sql/aggregates/basic.rs

@@ -103,6 +103,21 @@ async fn count_aggregated() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn count_aggregated_group_by_all_alias() -> Result<()> {
+    let results =
+        execute_with_partition("SELECT count(*) AS c FROM test GROUP BY ALL", 4).await?;
+
+    assert_snapshot!(batches_to_sort_string(&results), @r"
+    +----+
+    | c  |
+    +----+
+    | 40 |
+    +----+
+    ");
+    Ok(())
+}
+
 #[tokio::test]
 async fn count_aggregated_cube() -> Result<()> {
     let results = execute_with_partition(

datafusion/sql/src/select.rs

@@ -31,7 +31,7 @@ use datafusion_common::error::DataFusionErrorBuilder;
 use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion};
 use datafusion_common::{Column, DFSchema, Result, not_impl_err, plan_err};
 use datafusion_common::{RecursionUnnestOption, UnnestOptions};
-use datafusion_expr::expr::{Alias, PlannedReplaceSelectItem, WildcardOptions};
+use datafusion_expr::expr::{PlannedReplaceSelectItem, WildcardOptions};
 use datafusion_expr::expr_rewriter::{
     normalize_col, normalize_col_with_schemas_and_ambiguity_check, normalize_sorts,
 };
@@ -192,16 +192,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                 })
                 .collect::<Result<Vec<Expr>>>()?
         } else {
-            // 'group by all' groups wrt. all select expressions except 'AggregateFunction's.
-            // Filter and collect non-aggregate select expressions
+            // 'group by all' groups wrt. all select expressions except aggregate expressions.
+            // Note: aggregate expressions can be nested under one or more aliases
+            // (e.g. count(1) AS count(*) AS c), so detect aggregates recursively.
             select_exprs
                 .iter()
-                .filter(|select_expr| match select_expr {
-                    Expr::AggregateFunction(_) => false,
-                    Expr::Alias(Alias { expr, name: _, .. }) => {
-                        !matches!(**expr, Expr::AggregateFunction(_))
-                    }
-                    _ => true,
+                .filter(|select_expr| {
+                    find_aggregate_exprs(std::iter::once(*select_expr)).is_empty()
                 })
                 .cloned()
                 .collect()

datafusion/sql/tests/sql_integration.rs

@@ -1748,6 +1748,32 @@ fn select_simple_aggregate_with_groupby_can_use_alias() {
     );
 }
 
+#[test]
+fn select_count_with_group_by_all() {
+    let plan = logical_plan("SELECT COUNT(*) FROM person GROUP BY ALL").unwrap();
+    assert_snapshot!(
+        plan,
+        @r"
+    Projection: count(*)
+      Aggregate: groupBy=[[]], aggr=[[count(*)]]
+        TableScan: person
+    "
+    );
+}
+
+#[test]
+fn select_count_alias_with_group_by_all() {
+    let plan = logical_plan("SELECT COUNT(*) AS c FROM person GROUP BY ALL").unwrap();
+    assert_snapshot!(
+        plan,
+        @r"
+    Projection: count(*) AS c
+      Aggregate: groupBy=[[]], aggr=[[count(*)]]
+        TableScan: person
+    "
+    );
+}
+
 #[test]
 fn select_simple_aggregate_with_groupby_aggregate_repeated() {
     let sql = "SELECT state, MIN(age), MIN(age) FROM person GROUP BY state";