Display all partitions and files in EXPLAIN VERBOSE #6711
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qrilka
force-pushed
the
all-file-groups-in-explain-verbose
branch
from
53f5a27
to
97deba0
Compare
Adds DisplayAs trait for structs which could show more details when formatted in the verbose mode Resolves apache#6383
qrilka
force-pushed
the
all-file-groups-in-explain-verbose
branch
from
97deba0
to
7d74efe
Compare
alamb
approved these changes
Jun 19, 2023
There was a problem hiding this comment.
Thank you @qrilka -- I think this code is both well structured and easy to understand, and well tested 🏅
cc @liurenjie1024 this might impact #3606
I also tested this locally and it worked great
❯ explain select client_id from '/tmp/test';
+---------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type | plan |
+---------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | TableScan: /tmp/test projection=[client_id] |
| physical_plan | ParquetExec: file_groups={10 groups: [[private/tmp/test/0002a768-11ec-4f79-873b-b6d9ea3338e2.parquet], [private/tmp/test/00106ee6-f64c-4a74-abae-7ea2c07d1ae7.parquet], [private/tmp/test/0009fce5-b15b-430f-8771-cb38c97ca33f.parquet], [private/tmp/test/00051c33-c0cd-467f-893a-fbf5ba916426.parquet], [private/tmp/test/00043ab8-c5fb-4fc3-8b2c-180608581b13.parquet], ...]}, projection=[client_id] |
| | |
+---------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
2 rows in set. Query took 0.395 seconds.
❯ explain verbose select client_id from '/tmp/test';
+------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type | plan |
+------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| initial_logical_plan | Projection: /tmp/test.client_id |
| | TableScan: /tmp/test |
| logical_plan after inline_table_scan | SAME TEXT AS ABOVE |
| logical_plan after type_coercion | SAME TEXT AS ABOVE |
| logical_plan after count_wildcard_rule | SAME TEXT AS ABOVE |
| analyzed_logical_plan | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after unwrap_cast_in_comparison | SAME TEXT AS ABOVE |
| logical_plan after replace_distinct_aggregate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_join | SAME TEXT AS ABOVE |
| logical_plan after decorrelate_predicate_subquery | SAME TEXT AS ABOVE |
| logical_plan after scalar_subquery_to_join | SAME TEXT AS ABOVE |
| logical_plan after extract_equijoin_predicate | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after merge_projection | SAME TEXT AS ABOVE |
| logical_plan after rewrite_disjunctive_predicate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_duplicated_expr | SAME TEXT AS ABOVE |
| logical_plan after eliminate_filter | SAME TEXT AS ABOVE |
| logical_plan after eliminate_cross_join | SAME TEXT AS ABOVE |
| logical_plan after common_sub_expression_eliminate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_limit | SAME TEXT AS ABOVE |
| logical_plan after propagate_empty_relation | SAME TEXT AS ABOVE |
| logical_plan after filter_null_join_keys | SAME TEXT AS ABOVE |
| logical_plan after eliminate_outer_join | SAME TEXT AS ABOVE |
| logical_plan after push_down_limit | SAME TEXT AS ABOVE |
| logical_plan after push_down_filter | SAME TEXT AS ABOVE |
| logical_plan after single_distinct_aggregation_to_group_by | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after unwrap_cast_in_comparison | SAME TEXT AS ABOVE |
| logical_plan after common_sub_expression_eliminate | SAME TEXT AS ABOVE |
| logical_plan after push_down_projection | Projection: /tmp/test.client_id |
| | TableScan: /tmp/test projection=[client_id] |
| logical_plan after eliminate_projection | TableScan: /tmp/test projection=[client_id] |
| logical_plan after push_down_limit | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after unwrap_cast_in_comparison | SAME TEXT AS ABOVE |
| logical_plan after replace_distinct_aggregate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_join | SAME TEXT AS ABOVE |
| logical_plan after decorrelate_predicate_subquery | SAME TEXT AS ABOVE |
| logical_plan after scalar_subquery_to_join | SAME TEXT AS ABOVE |
| logical_plan after extract_equijoin_predicate | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after merge_projection | SAME TEXT AS ABOVE |
| logical_plan after rewrite_disjunctive_predicate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_duplicated_expr | SAME TEXT AS ABOVE |
| logical_plan after eliminate_filter | SAME TEXT AS ABOVE |
| logical_plan after eliminate_cross_join | SAME TEXT AS ABOVE |
| logical_plan after common_sub_expression_eliminate | SAME TEXT AS ABOVE |
| logical_plan after eliminate_limit | SAME TEXT AS ABOVE |
| logical_plan after propagate_empty_relation | SAME TEXT AS ABOVE |
| logical_plan after filter_null_join_keys | SAME TEXT AS ABOVE |
| logical_plan after eliminate_outer_join | SAME TEXT AS ABOVE |
| logical_plan after push_down_limit | SAME TEXT AS ABOVE |
| logical_plan after push_down_filter | SAME TEXT AS ABOVE |
| logical_plan after single_distinct_aggregation_to_group_by | SAME TEXT AS ABOVE |
| logical_plan after simplify_expressions | SAME TEXT AS ABOVE |
| logical_plan after unwrap_cast_in_comparison | SAME TEXT AS ABOVE |
| logical_plan after common_sub_expression_eliminate | SAME TEXT AS ABOVE |
| logical_plan after push_down_projection | SAME TEXT AS ABOVE |
| logical_plan after eliminate_projection | SAME TEXT AS ABOVE |
| logical_plan after push_down_limit | SAME TEXT AS ABOVE |
| logical_plan | TableScan: /tmp/test projection=[client_id] |
| initial_physical_plan | ParquetExec: file_groups={10 groups: [[private/tmp/test/0002a768-11ec-4f79-873b-b6d9ea3338e2.parquet], [private/tmp/test/00106ee6-f64c-4a74-abae-7ea2c07d1ae7.parquet], [private/tmp/test/0009fce5-b15b-430f-8771-cb38c97ca33f.parquet], [private/tmp/test/00051c33-c0cd-467f-893a-fbf5ba916426.parquet], [private/tmp/test/00043ab8-c5fb-4fc3-8b2c-180608581b13.parquet], [private/tmp/test/000976e7-97c3-4b91-9d8f-d3fc7c328630.parquet], [private/tmp/test/0011dc97-b8ec-44af-a373-c8b0617000da.parquet], [private/tmp/test/000508ea-3512-4f80-8d09-8aaa41f95d00.parquet], [private/tmp/test/0004c666-9e0d-4a01-8a1e-85cd891ae9eb.parquet], [private/tmp/test/001694dd-012c-4b72-9a3e-ba92422e250d.parquet]]}, projection=[client_id] |
| | |
| physical_plan after aggregate_statistics | SAME TEXT AS ABOVE |
| physical_plan after join_selection | SAME TEXT AS ABOVE |
| physical_plan after PipelineFixer | SAME TEXT AS ABOVE |
| physical_plan after repartition | SAME TEXT AS ABOVE |
| physical_plan after global_sort_selection | SAME TEXT AS ABOVE |
| physical_plan after EnforceDistribution | SAME TEXT AS ABOVE |
| physical_plan after CombinePartialFinalAggregate | SAME TEXT AS ABOVE |
| physical_plan after EnforceSorting | SAME TEXT AS ABOVE |
| physical_plan after coalesce_batches | SAME TEXT AS ABOVE |
| physical_plan after PipelineChecker | SAME TEXT AS ABOVE |
| physical_plan | ParquetExec: file_groups={10 groups: [[private/tmp/test/0002a768-11ec-4f79-873b-b6d9ea3338e2.parquet], [private/tmp/test/00106ee6-f64c-4a74-abae-7ea2c07d1ae7.parquet], [private/tmp/test/0009fce5-b15b-430f-8771-cb38c97ca33f.parquet], [private/tmp/test/00051c33-c0cd-467f-893a-fbf5ba916426.parquet], [private/tmp/test/00043ab8-c5fb-4fc3-8b2c-180608581b13.parquet], [private/tmp/test/000976e7-97c3-4b91-9d8f-d3fc7c328630.parquet], [private/tmp/test/0011dc97-b8ec-44af-a373-c8b0617000da.parquet], [private/tmp/test/000508ea-3512-4f80-8d09-8aaa41f95d00.parquet], [private/tmp/test/0004c666-9e0d-4a01-8a1e-85cd891ae9eb.parquet], [private/tmp/test/001694dd-012c-4b72-9a3e-ba92422e250d.parquet]]}, projection=[client_id] |
| | |
+------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
72 rows in set. Query took 0.398 seconds.| 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 |
There was a problem hiding this 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 {
...
}
There was a problem hiding this comment.
Sure but why do you think it won't be trivial? If rust-lang/rust#31844 stabilized it would be much easier of course but as far as I see, the change is tedious but should be straitforward: just split fmt_as into as separate impl. Do I miss anything here?
There was a problem hiding this comment.
Sure but why do you think it won't be trivial? If rust-lang/rust#31844 stabilized it would be much easier of course but as far as I see, the change is tedious but should be straitforward: just split fmt_as into as separate impl. Do I miss anything here?
I agree the change would be mechanical (to move fmt_as into a separate impl), but it would be required for all impl ExecutionPlan impls which is why I would I not call it "trivial" -- but i agree that is just my opinion and how others might have a different opinion
| @@ -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 | |||
| @@ -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)); | |||
|
I merged up from main and plan to merge this PR once it passes the tests |
|
Thanks again @qrilka |
|
Sure, I guess it makes sense to create a follow-up PR with |
Yes, this is what I think makes the most sense -- thank you |
qrilka
added a commit
to qrilka/arrow-datafusion
that referenced
this pull request
Jul 3, 2023
qrilka
added a commit
to qrilka/arrow-datafusion
that referenced
this pull request
Jul 3, 2023
alamb
pushed a commit
that referenced
this pull request
Jul 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds DisplayAs trait for structs which could show more details when formatted in the verbose mode
Resolves #6383
Which issue does this PR close?
Closes #6383.
What changes are included in this PR?
Notes about the implementation:
physical_plan::display::DisplayAswas created similar tophysical_plan::ExecutionPlan::fmt_as(but this method is left in its current trait)verbosemode wasn't actually used in the code and to make it work I addedverboseas an argument ofphysical_plan::display::DisplayableExecutionPlan::indentand movedto_stringifiedinto that trait and also addedverboseinto this methodfmt_asin two different traits and also 2 slightly differentto_stringifiedin two traits as well but I couldn't find a good way around thatverbosewasn't actually used)Are these changes tested?
New unit tests were added. Plus manual test from the ticket also passes
Are there any user-facing changes?
The docs don't have much details about
EXPLAINso I'm not sure that we need to add this minor detail.