You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/08/16 14:43:33 UTC
[PR] Move PruningPredicate to physical-optimizer crate [datafusion]
alamb opened a new pull request, #12037:
URL: https://github.com/apache/datafusion/pull/12037
## Which issue does this PR close?
Part of https://github.com/apache/datafusion/issues/11502
## Rationale for this change
We are trying to break up pieces the `datafusion` crate into smaller pieces for faster compile time and better modularity. Part of this is the physical optimizer
## What changes are included in this PR?
Move `PruningPredicate` into the physical optimizer crate
## Are these changes tested?
Yes, by existing CI
## Are there any user-facing changes?
No, there are backwards compatible `pub use` that mean no user change are required
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Move PruningPredicate to physical-optimizer crate [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12037:
URL: https://github.com/apache/datafusion/pull/12037#discussion_r1719949349
##########
datafusion/core/src/physical_optimizer/mod.rs:
##########
@@ -29,7 +29,6 @@ pub mod join_selection;
pub mod limited_distinct_aggregation;
pub mod optimizer;
pub mod projection_pushdown;
-pub mod pruning;
Review Comment:
there is a
```rust
pub use datafusion_physical_optimizer::*;
```
a few lines down which adds an alias to the `pruning` module in its new home
##########
datafusion/physical-optimizer/src/lib.rs:
##########
@@ -23,3 +23,5 @@ mod optimizer;
pub mod output_requirements;
pub use optimizer::PhysicalOptimizerRule;
+
+pub mod pruning;
Review Comment:
this was just to make the `use` statements less ugly
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -615,7 +611,8 @@ impl PruningPredicate {
is_always_true(&self.predicate_expr) && self.literal_guarantees.is_empty()
}
- pub(crate) fn required_columns(&self) -> &RequiredColumns {
+ /// Return the columns that are required to evaluate the pruning predicate
+ pub fn required_columns(&self) -> &RequiredColumns {
Review Comment:
as it is in a different crate now this function needs to become pub, which I think is reasonable as it is part of the API
Likewise the change to make `RequiredColumns` pub
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Move PruningPredicate to physical-optimizer crate [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12037:
URL: https://github.com/apache/datafusion/pull/12037#discussion_r1719946588
##########
datafusion/expr-common/src/lib.rs:
##########
@@ -17,9 +17,13 @@
//! Logical Expr types and traits for [DataFusion]
//!
-//! This crate contains types and traits that are used by both Logical and Physical expressions.
-//! They are kept in their own crate to avoid physical expressions depending on logical expressions.
-//!
+//! This crate contains types and traits that are used by both Logical and
+//! Physical expressions. They are kept in their own crate to avoid physical
+//! expressions depending on logical expressions.
+//!
+//! Note this crate is not intended to have substantial logic itself, but rather
Review Comment:
I added some additional explanation about this crate (partly as a justification about why it was ok to depend on in the physical optimizer)
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Move PruningPredicate to physical-optimizer crate [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #12037:
URL: https://github.com/apache/datafusion/pull/12037#issuecomment-2293674482
I need to update the tests
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Move PruningPredicate to physical-optimizer crate [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed pull request #12037: Move PruningPredicate to physical-optimizer crate
URL: https://github.com/apache/datafusion/pull/12037
--
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@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org