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/03/23 05:30:52 UTC
[GitHub] [arrow-datafusion] Ted-Jiang opened a new pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Ted-Jiang opened a new pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063
# Which issue does this PR close?
Closes #2021.
# Rationale for this change
Thanks @alamb 's desgin to decouple between explain and planNode, No need to change `tableScan` struct.
Now
```
❯ explain select c1, c2 from test where c2 = 0.000001;
+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type | plan |
+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | Projection: #test.c1, #test.c2 |
| | Filter: #test.c2 = Float64(0.000001) |
| | TableScan: test projection=Some([0, 1]), partial_filters=[#test.c2 = Float64(0.000001)] |
| physical_plan | ProjectionExec: expr=[c1@0 as c1, c2@1 as c2] |
| | CoalesceBatchesExec: target_batch_size=4096 |
| | FilterExec: CAST(c2@1 AS Float64) = 0.000001 |
| | RepartitionExec: partitioning=RoundRobinBatch(16) |
| | CsvExec: files=[/Users/yangjiang/CLionProjects/github/arrow-datafusion/testing/data/csv/aggregate_test_100.csv], has_header=true, limit=None |
| | |
+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
2 rows in set. Query took 0.004 seconds.
```
# What changes are included in this PR?
plan tree node `TableScan`
# Are there any user-facing changes?
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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] yjshen merged pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen merged pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063
--
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 #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1076745055
> We lost filters and projections in ParquetExec, lost projections in CSVExec, etc. What do you think if we include these missing parts for physical scan operators as well?
I don't fully understand what you are suggesting here @yjshen -- are you suggesting:
1. That this PR loses some information?
2. A follow on PR to add more information to the `ParquetExec` would be helpful?
3. Something else?
Thanks again @Ted-Jiang -- this looks very helpful to avoid confusion
--
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] yjshen commented on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen commented on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1075968812
We lost filters and projections in `ParquetExec`, lost projections for `CSV`, etc. What do you think if we include these missing parts for physical scan operators as well?
--
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] Ted-Jiang commented on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
Ted-Jiang commented on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1076154991
> We lost filters and projections in `ParquetExec`, lost projections in `CSVExec`, etc. What do you think if we include these missing parts for physical scan operators as well?
Sure! may i create a sperate issue for these physican plan fmt change (avoid blend them together).
--
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] yjshen edited a comment on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen edited a comment on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1076959321
> A follow on PR to add more information to the ParquetExec would be helpful?
Yes, Physical Table Scan Operators (ParquetExec, CsvExec ...) are losing information such as projection and predicate while being explained. We could be in a follow-up PR.
--
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] yjshen commented on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen commented on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1076959321
> A follow on PR to add more information to the ParquetExec would be helpful?
Yes, Physical Table Scan Operators (ParquetExec, CsvExec ...) are losing information such as projection and predicate while being explained. We could be in a follow-up PR.
--
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] yjshen edited a comment on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen edited a comment on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1075968812
We lost filters and projections in `ParquetExec`, lost projections in `CSVExec`, etc. What do you think if we include these missing parts for physical scan operators as well?
--
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] yjshen edited a comment on pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
yjshen edited a comment on pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#issuecomment-1075968812
We lost filters and projections in `ParquetExec`, lost projections for `CSVExec`, etc. What do you think if we include these missing parts for physical scan operators as well?
--
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 change in pull request #2063: Change the DataFusion explain plans to make it clearer in the predicate/filter
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #2063:
URL: https://github.com/apache/arrow-datafusion/pull/2063#discussion_r833653731
##########
File path: datafusion/src/logical_plan/plan.rs
##########
@@ -913,7 +916,39 @@ impl LogicalPlan {
)?;
if !filters.is_empty() {
- write!(f, ", filters={:?}", filters)?;
+ let mut full_filter = vec![];
Review comment:
this is quite nice
##########
File path: datafusion/src/optimizer/filter_push_down.rs
##########
@@ -1431,7 +1431,7 @@ mod tests {
let plan = table_scan_with_pushdown_provider(TableProviderFilterPushDown::Exact)?;
let expected = "\
- TableScan: test projection=None, filters=[#a = Int64(1)]";
+ TableScan: test projection=None, full_filters=[#a = Int64(1)]";
Review comment:
👍
--
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