Skip to content

Commit cc96026

Browse files
authored
Allow aggregation without projection in Unparser (#13326)
* Allow aggregation without projection in Unparser * Fix formatting
1 parent 99cdbcc commit cc96026

File tree

2 files changed

+50
-2
lines changed

2 files changed

+50
-2
lines changed

datafusion/sql/src/unparser/plan.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,27 @@ impl Unparser<'_> {
420420
)
421421
}
422422
LogicalPlan::Aggregate(agg) => {
423-
// Aggregate nodes are handled simultaneously with Projection nodes
423+
// Aggregation can be already handled in the projection case
424+
if !select.already_projected() {
425+
// The query returns aggregate and group expressions. If that weren't the case,
426+
// the aggregate would have been placed inside a projection, making the check above^ false
427+
let exprs: Vec<_> = agg
428+
.aggr_expr
429+
.iter()
430+
.chain(agg.group_expr.iter())
431+
.map(|expr| self.select_item_to_sql(expr))
432+
.collect::<Result<Vec<_>>>()?;
433+
select.projection(exprs);
434+
435+
select.group_by(ast::GroupByExpr::Expressions(
436+
agg.group_expr
437+
.iter()
438+
.map(|expr| self.expr_to_sql(expr))
439+
.collect::<Result<Vec<_>>>()?,
440+
vec![],
441+
));
442+
}
443+
424444
self.select_to_sql_recursively(
425445
agg.input.as_ref(),
426446
query,

datafusion/sql/tests/cases/plan_to_sql.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ use std::vec;
2020

2121
use arrow_schema::*;
2222
use datafusion_common::{DFSchema, Result, TableReference};
23-
use datafusion_expr::test::function_stub::{count_udaf, max_udaf, min_udaf, sum_udaf};
23+
use datafusion_expr::test::function_stub::{
24+
count_udaf, max_udaf, min_udaf, sum, sum_udaf,
25+
};
2426
use datafusion_expr::{col, lit, table_scan, wildcard, LogicalPlanBuilder};
2527
use datafusion_functions::unicode;
2628
use datafusion_functions_aggregate::grouping::grouping_udaf;
@@ -563,6 +565,32 @@ Projection: unnest_placeholder(unnest_table.struct_col).field1, unnest_placehold
563565
Ok(())
564566
}
565567

568+
#[test]
569+
fn test_aggregation_without_projection() -> Result<()> {
570+
let schema = Schema::new(vec![
571+
Field::new("name", DataType::Utf8, false),
572+
Field::new("age", DataType::UInt8, false),
573+
]);
574+
575+
let plan = LogicalPlanBuilder::from(
576+
table_scan(Some("users"), &schema, Some(vec![0, 1]))?.build()?,
577+
)
578+
.aggregate(vec![col("name")], vec![sum(col("age"))])?
579+
.build()?;
580+
581+
let unparser = Unparser::default();
582+
let statement = unparser.plan_to_sql(&plan)?;
583+
584+
let actual = &statement.to_string();
585+
586+
assert_eq!(
587+
actual,
588+
r#"SELECT sum(users.age), users."name" FROM (SELECT users."name", users.age FROM users) GROUP BY users."name""#
589+
);
590+
591+
Ok(())
592+
}
593+
566594
#[test]
567595
fn test_table_references_in_plan_to_sql() {
568596
fn test(table_name: &str, expected_sql: &str, dialect: &impl UnparserDialect) {

0 commit comments

Comments
 (0)