findepi commented on code in PR #17819:
URL: https://github.com/apache/datafusion/pull/17819#discussion_r2390264026
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -530,14 +530,14 @@ fn push_down_join(
parent_predicate: Option<&Expr>,
) -> Result<Transformed<LogicalPlan>> {
// Split the parent predicate into individual conjunctive parts.
- let predicates = parent_predicate
- .map_or_else(Vec::new, |pred| split_conjunction_owned(pred.clone()));
+ let predicates =
Review Comment:
> this certainly looks nicer but it seems like the expressions are still
cloned at the end.
Correct. The intention was to delay cloning, but I admit the `Avoid clones
in push_down_join` commit is not ideal. I realized that we avoid cloning only
in case there wasn't much to clone in the first place. I will revert the commit.
What I wanted to show with this commit is that implementing `AsRef<Expr> for
Expr` allows writing APIs that are impossible to model otherwise. Note also
that downstream projects have no way to add such impl or workaround lack of it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
