alamb commented on code in PR #15103:
URL: https://github.com/apache/datafusion/pull/15103#discussion_r1986313731
##########
datafusion/sqllogictest/test_files/explain_tree.slt:
##########
@@ -124,25 +124,39 @@ logical_plan
physical_plan
01)┌───────────────────────────┐
02)│ AggregateExec │
-03)└─────────────┬─────────────┘
-04)┌─────────────┴─────────────┐
-05)│ CoalesceBatchesExec │
-06)└─────────────┬─────────────┘
-07)┌─────────────┴─────────────┐
-08)│ RepartitionExec │
-09)└─────────────┬─────────────┘
-10)┌─────────────┴─────────────┐
-11)│ AggregateExec │
-12)└─────────────┬─────────────┘
-13)┌─────────────┴─────────────┐
-14)│ RepartitionExec │
-15)└─────────────┬─────────────┘
-16)┌─────────────┴─────────────┐
-17)│ DataSourceExec │
-18)│ -------------------- │
-19)│ files: 1 │
-20)│ format: csv │
-21)└───────────────────────────┘
+03)│ -------------------- │
+04)│ aggr: │
+05)│ sum(table1.bigint_col) │
+06)│ │
+07)│ group_by: @0 │
Review Comment:
why is this showing `@0`?
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -809,8 +809,54 @@ impl DisplayAs for AggregateExec {
}
}
DisplayFormatType::TreeRender => {
- // TODO: collect info
- write!(f, "")?;
+ let g: Vec<String> = if self.group_by.is_single() {
Review Comment:
there seems to be quite a bit of formatting code here -- but many of the
cases are not covered by the tests
can you explain what it is trying to do?
Perhaps you are trying to make the expressions better to read, similar to
- https://github.com/apache/datafusion/issues/15107
What I suggest for this PR is:
1. Follow the existing patterns for `tree` mode
2. Ensure any important case is covered by tests
And then we can work on making expression printing nicer as a follow on PR
(and so it applies to all `tree` modes)
--
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]
