alamb commented on code in PR #10767: URL: https://github.com/apache/datafusion/pull/10767#discussion_r1623495522
########## datafusion/sql/src/unparser/plan.rs: ########## @@ -162,23 +162,40 @@ impl Unparser<'_> { // A second projection implies a derived tablefactor if !select.already_projected() { // Special handling when projecting an agregation plan - if let Some(agg) = find_agg_node_within_select(plan, true) { - let items = p - .expr - .iter() - .map(|proj_expr| { - let unproj = unproject_agg_exprs(proj_expr, agg)?; - self.select_item_to_sql(&unproj) - }) - .collect::<Result<Vec<_>>>()?; - - select.projection(items); - select.group_by(ast::GroupByExpr::Expressions( - agg.group_expr - .iter() - .map(|expr| self.expr_to_sql(expr)) - .collect::<Result<Vec<_>>>()?, - )); + if let Some(aggvariant) = find_agg_node_within_select(plan, true) { Review Comment: I agree this would be a good follow on PR. ########## datafusion/sql/tests/cases/plan_to_sql.rs: ########## @@ -127,7 +127,10 @@ fn roundtrip_statement() -> Result<()> { UNION ALL SELECT j2_string as string FROM j2 ORDER BY string DESC - LIMIT 10"# + LIMIT 10"#, + "SELECT id, count(*) over (PARTITION BY first_name ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING), Review Comment: > While the two plans are not identical p1!=p2, I believe the difference is trivial and will actually result in the same computations taking place. That is my understanding as well We could plausibly simply the resulting plan of the window bounds are the default Can we also add some tests that have aggregate and window functions? Something like ```sql SELECT id, count(distinct id), sum(id) OVER (PARTITION BY first_name) from person SELECT id, sum(id) OVER (PARTITION BY first_name ROWS 5 PRECEDING ROWS 2 FOLLOWING) from person ``` -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org