alamb commented on code in PR #6711:
URL: https://github.com/apache/arrow-datafusion/pull/6711#discussion_r1234109250
##########
datafusion/core/src/physical_plan/display.rs:
##########
@@ -30,6 +30,8 @@ use super::{accept, ExecutionPlan, ExecutionPlanVisitor};
pub enum DisplayFormatType {
/// Default, compact format. Example: `FilterExec: c12 < 10.0`
Default,
+ /// Verbose, showing all available details
Review Comment:
👍
##########
datafusion/substrait/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -66,8 +66,11 @@ async fn parquet_exec() -> Result<()> {
consumer::from_substrait_rel(&mut ctx, substrait_rel.as_ref(),
&HashMap::new())
.await?;
- let expected = format!("{}", displayable(parquet_exec.as_ref()).indent());
- let actual = format!("{}",
displayable(parquet_exec_roundtrip.as_ref()).indent());
+ let expected = format!("{}",
displayable(parquet_exec.as_ref()).indent(true));
Review Comment:
💯 for including verbose
##########
datafusion/core/src/physical_plan/display.rs:
##########
@@ -192,11 +209,29 @@ impl<'a, 'b> ExecutionPlanVisitor for IndentVisitor<'a,
'b> {
}
}
-impl<'a> ToStringifiedPlan for DisplayableExecutionPlan<'a> {
- fn to_stringified(
- &self,
- plan_type: crate::logical_expr::PlanType,
- ) -> StringifiedPlan {
- StringifiedPlan::new(plan_type, self.indent().to_string())
+/// Trait for types which could have additional details when formatted in
`Verbose` mode
Review Comment:
🤔 maybe eventually we can combine this into `ExecutionPlan` so that
everything that implemented `ExecutionPlan` would also need to implement
`DisplayAs`.
That would reduce the duplication across the traits, howe er, it would be an
API change and result in non trivial code churn so I think we can consider it
as part of a follow on PR
```
trait ExecutionPlan: DisplayAs {
...
}
```
--
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]
