-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans. #2895
Conversation
…e, the explain statement will only print logical plans.
Codecov Report
@@ Coverage Diff @@
## master #2895 +/- ##
=======================================
Coverage 85.34% 85.34%
=======================================
Files 276 276
Lines 49294 49296 +2
=======================================
+ Hits 42071 42073 +2
Misses 7223 7223
Continue to review full report at Codecov.
|
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.
Thank you @AssHero -- This looks good to me
I think this feature needs a test, so this feature isn't accidentally broken in the future, but extending one of the existing tests should be straight forward
datafusion/core/src/config.rs
Outdated
@@ -27,6 +27,9 @@ use std::env; | |||
/// Configuration option "datafusion.optimizer.filter_null_join_keys" | |||
pub const OPT_FILTER_NULL_JOIN_KEYS: &str = "datafusion.optimizer.filter_null_join_keys"; | |||
|
|||
/// Configuration option "datafusion.explain.logical_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.
/// Configuration option "datafusion.explain.logical_plan" | |
/// Configuration option "datafusion.explain.logical_plan_only" |
Maybe we could call it datafusion.explain.logical_plan_only
to be consistent with the internal implementation and be more self describing
Another way to encode this might be "datafusion.explain.include_physical_plan", defaulted to true. That way we could also encode a "datafusion.explain.include_logical_plan" if people ever wanted only the physical plan
This way works too. problem with this way, however
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.
Thanks!
I add two options 'datafusion.explain.logical_plan_only' and 'datafusion.explain.physical_plan_only'.
If set datafusion.explain.logical_plan_only to true, only print logical plans.
If set datafusion.explain.physical_plan_only to true, only print physical plans.
Both default to false.
But if both are set to true, print nothing currently.
I'll add a test later. Thanks! |
…plan_only' and 'datafusion.explain.physical_plan_only' to seperately print logical plans and physical plans.
|
@@ -119,6 +125,16 @@ impl BuiltInConfigs { | |||
predicate push down.", | |||
false, | |||
), | |||
ConfigDefinition::new_bool( |
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.
can you run ./dev/update_config_docs.sh
to re-generate the documentation to include these new options
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.
The docs/source/user-guide/configs.md is updated. Thanks!
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.
looks good to me 👍
Re-generate the documentation to include these new options. |
👍 Thanks again @AssHero ! |
Benchmark runs are scheduled for baseline = 6dc9dea and contender = 1b8117e. 1b8117e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2894
Rationale for this change
Add session option 'datafusion.explain.logical_plan'.
when set to true, the explain statement will only print logical plans. We can set environment variable DATAFUSION_EXPLAIN_LOGICAL_PLAN to make this value take effect.
For example:
explain select count(*) from (values ('a', 1, 100), ('a', 2, 150)) as t (c1,c2,c3);
+--------------+----------------------------------------------------------------------------------+
| plan_type | plan |
+--------------+----------------------------------------------------------------------------------+
| logical_plan | Projection: #COUNT(UInt8(1)) |
| | Aggregate: groupBy=[[]], aggr=[[COUNT(UInt8(1))]] |
| | Values: (Utf8("a"), Int64(1), Int64(100)), (Utf8("a"), Int64(2), Int64(150)) |
+--------------+----------------------------------------------------------------------------------+
What changes are included in this PR?
add session option 'datafusion.explain.logical_plan'. in datafusion/core/src/config.rs
use this option to control the output of explain statement in datafusion/core/src/physical_plan/planner.rs