-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Show the result of all optimizer passes in EXPLAIN VERBOSE #759
Conversation
_, | ||
) => { | ||
let schema = schema.as_ref().to_owned().into(); | ||
optimize_explain( |
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.
this is the old style "explain" implementation that needed to be done for each optimizer for it to correctly show explain plans (and it was missing from several)
plan_builder.append_value(&*p.plan)?; | ||
match prev { | ||
Some(prev) if !should_show(prev, p) => { | ||
plan_builder.append_value("SAME TEXT AS ABOVE")?; |
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.
Once I started dumping out all the explain plans, the mount of replication was enormous, so I also added code to avoid duplication if the optimizer pass did not make any changes
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.
+1
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.
LGTM
schema: schema.clone(), | ||
}) | ||
} else { | ||
self.optimize_internal(plan, |_, _| {}) |
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.
Nice. Now we have the optimized plan displayed
plan_builder.append_value(&*p.plan)?; | ||
match prev { | ||
Some(prev) if !should_show(prev, p) => { | ||
plan_builder.append_value("SAME TEXT AS ABOVE")?; |
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.
+1
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.
LGTM
Which issue does this PR close?
Resolves #733
Rationale for this Change
Previously, only some logical optimizer passes (and no physical optimizer passes) were shown in
EXPLAIN VERBOSE
output. This was due to the fact that each optimizer had to special case handling for explain and so unsurprisingly some (especially newly written ones) did not.What changes are included in this PR?
ExecutionContext::Optimize
Are there any user-facing changes?
Yes. Explain output is different. To see the difference, do
Then run
Before this change:
Note the reason the optimizer passes appear to be duplicated in this explain is because that is what actually happens -- optimize is called once as part of
ExecutionContext::sql()
and again as part ofDataFrame_impl::collect()
). If we want to avoid the double optimization, I think we should treat it separately and do so in a follow on PR. This PR faithfully captures what DataFusion is actually doing.After this change: