You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/13 07:06:12 UTC
[GitHub] [arrow-datafusion] AssHero opened a new pull request, #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
AssHero opened a new pull request, #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895
# 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
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] AssHero commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
AssHero commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1183120110
> 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
I'll add a test later. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] AssHero commented on a diff in pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
AssHero commented on code in PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#discussion_r920050062
##########
datafusion/core/src/config.rs:
##########
@@ -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"
Review Comment:
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.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1184277172
👍 Thanks again @AssHero !
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] codecov-commenter commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1182874277
# [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2895](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a47b237) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/6a5de4fe08597896ab6375e3e4b76c5744dcfba7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6a5de4f) will **increase** coverage by `0.00%`.
> The diff coverage is `92.30%`.
> :exclamation: Current head a47b237 differs from pull request most recent head e8551bc. Consider uploading reports for the commit e8551bc to get more accurate results
```diff
@@ Coverage Diff @@
## master #2895 +/- ##
=======================================
Coverage 85.34% 85.34%
=======================================
Files 276 276
Lines 49294 49296 +2
=======================================
+ Hits 42071 42073 +2
Misses 7223 7223
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [datafusion/core/src/config.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2895/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9jb25maWcucnM=) | `92.50% <ø> (ø)` | |
| [datafusion/core/src/physical\_plan/planner.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2895/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL3BsYW5uZXIucnM=) | `81.02% <92.30%> (+0.03%)` | :arrow_up: |
| [datafusion/physical-expr/src/expressions/binary.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2895/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9iaW5hcnkucnM=) | `95.03% <0.00%> (-0.09%)` | :arrow_down: |
| [datafusion/core/src/physical\_plan/metrics/value.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2895/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL21ldHJpY3MvdmFsdWUucnM=) | `87.43% <0.00%> (+0.50%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6a5de4f...e8551bc](https://codecov.io/gh/apache/arrow-datafusion/pull/2895?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] ursabot commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1184285251
Benchmark runs are scheduled for baseline = 6dc9dea30e496ad1c9880913abd73a90b6070fb2 and contender = 1b8117eaeca586e40882a80a53103f1be17d2b1c. 1b8117eaeca586e40882a80a53103f1be17d2b1c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/57d2d9dd251e4feeb52f88f3ccc14c65...106dba7acf6c4e71935a5b19115a9aee/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/dd9c55dafeca4d9daff66982e5595b52...43a0a0ebb19d4423920edfdbc769acd1/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/8eb49a162f6c44ae91279fea310b21b2...6d2f6bfb58774e83bc57c4a454bd68e2/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5658d21e7acd4a3594036eb65b2293ae...5ee087015af54062897437000116c51b/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#discussion_r919949364
##########
datafusion/core/src/config.rs:
##########
@@ -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"
Review Comment:
```suggestion
/// 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
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#discussion_r920433312
##########
datafusion/core/src/config.rs:
##########
@@ -119,6 +125,16 @@ impl BuiltInConfigs {
predicate push down.",
false,
),
+ ConfigDefinition::new_bool(
Review Comment:
can you run `./dev/update_config_docs.sh` to re-generate the documentation to include these new options
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
alamb merged PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] AssHero commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
AssHero commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1183195873
1. add test cases.
2. add session options 'datafusion.explain.logical_plan_only' and 'datafusion.explain.physical_plan_only' to seperately print logical plans and physical plans.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] AssHero commented on a diff in pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
AssHero commented on code in PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#discussion_r920651235
##########
datafusion/core/src/config.rs:
##########
@@ -119,6 +125,16 @@ impl BuiltInConfigs {
predicate push down.",
false,
),
+ ConfigDefinition::new_bool(
Review Comment:
The docs/source/user-guide/configs.md is updated. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] AssHero commented on pull request #2895: add session option 'datafusion.explain.logical_plan'. when set to true, the explain statement will only print logical plans.
Posted by GitBox <gi...@apache.org>.
AssHero commented on PR #2895:
URL: https://github.com/apache/arrow-datafusion/pull/2895#issuecomment-1183866941
Re-generate the documentation to include these new options.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org