-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for EXPLAIN ANALYZE #858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,6 +213,16 @@ pub enum LogicalPlan { | |
| /// The output schema of the explain (2 columns of text) | ||
| schema: DFSchemaRef, | ||
| }, | ||
| /// Runs the actual plan, and then prints the physical plan with | ||
| /// with execution metrics. | ||
| Analyze { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE: I chose a new LogicalPlan node because the implementation for ANALYZE is so different than EXPLAIN. However, it would be possible to re-use the same LogicalPlan node if people prefer
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am about to ask when I saw the above code that has analyze as different function. I am worry about future inconsistency and headache of keeping them consistent, as well as redundant work when we change or improve something. I would prefer to keep them in the same LogicalPlan |
||
| /// Should extra detail be included? | ||
| verbose: bool, | ||
| /// The logical plan that is being EXPLAIN ANALYZE'd | ||
| input: Arc<LogicalPlan>, | ||
| /// The output schema of the explain (2 columns of text) | ||
| schema: DFSchemaRef, | ||
| }, | ||
| /// Extension operator defined outside of DataFusion | ||
| Extension { | ||
| /// The runtime extension operator | ||
|
|
@@ -239,6 +249,7 @@ impl LogicalPlan { | |
| LogicalPlan::Limit { input, .. } => input.schema(), | ||
| LogicalPlan::CreateExternalTable { schema, .. } => schema, | ||
| LogicalPlan::Explain { schema, .. } => schema, | ||
| LogicalPlan::Analyze { schema, .. } => schema, | ||
| LogicalPlan::Extension { node } => node.schema(), | ||
| LogicalPlan::Union { schema, .. } => schema, | ||
| } | ||
|
|
@@ -278,6 +289,7 @@ impl LogicalPlan { | |
| } | ||
| LogicalPlan::Extension { node } => vec![node.schema()], | ||
| LogicalPlan::Explain { schema, .. } | ||
| | LogicalPlan::Analyze { schema, .. } | ||
| | LogicalPlan::EmptyRelation { schema, .. } | ||
| | LogicalPlan::CreateExternalTable { schema, .. } => vec![schema], | ||
| LogicalPlan::Limit { input, .. } | ||
|
|
@@ -327,6 +339,7 @@ impl LogicalPlan { | |
| | LogicalPlan::Limit { .. } | ||
| | LogicalPlan::CreateExternalTable { .. } | ||
| | LogicalPlan::CrossJoin { .. } | ||
| | LogicalPlan::Analyze { .. } | ||
| | LogicalPlan::Explain { .. } | ||
| | LogicalPlan::Union { .. } => { | ||
| vec![] | ||
|
|
@@ -350,6 +363,7 @@ impl LogicalPlan { | |
| LogicalPlan::Extension { node } => node.inputs(), | ||
| LogicalPlan::Union { inputs, .. } => inputs.iter().collect(), | ||
| LogicalPlan::Explain { plan, .. } => vec![plan], | ||
| LogicalPlan::Analyze { input: plan, .. } => vec![plan], | ||
| // plans without inputs | ||
| LogicalPlan::TableScan { .. } | ||
| | LogicalPlan::EmptyRelation { .. } | ||
|
|
@@ -495,6 +509,7 @@ impl LogicalPlan { | |
| true | ||
| } | ||
| LogicalPlan::Explain { plan, .. } => plan.accept(visitor)?, | ||
| LogicalPlan::Analyze { input: plan, .. } => plan.accept(visitor)?, | ||
| // plans without inputs | ||
| LogicalPlan::TableScan { .. } | ||
| | LogicalPlan::EmptyRelation { .. } | ||
|
|
@@ -790,6 +805,7 @@ impl LogicalPlan { | |
| write!(f, "CreateExternalTable: {:?}", name) | ||
| } | ||
| LogicalPlan::Explain { .. } => write!(f, "Explain"), | ||
| LogicalPlan::Analyze { .. } => write!(f, "Analyze"), | ||
| LogicalPlan::Union { .. } => write!(f, "Union"), | ||
| LogicalPlan::Extension { ref node } => node.fmt_for_explain(f), | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we talk I need to learn the the effectiveness of this round trip test thats convert a logical/physical plan into photo and back. The tests look simple and easy to understand this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest I simply copy/pasted the test for
roundtrip_explainbelow -- I agree the pattern is quite nice