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: [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]
