alamb commented on code in PR #10008:
URL:
https://github.com/apache/arrow-datafusion/pull/10008#discussion_r1557373552
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1321,6 +1321,20 @@ impl LogicalPlan {
| LogicalPlan::Extension(_) => None,
}
}
+
+ pub fn contains_outer_reference(&self) -> bool {
Review Comment:
If we are going to make this a public functions, I think it should have
comments. For example:
```suggestion
/// If this node's expressions contains any references to an outer
subquery
pub fn contains_outer_reference(&self) -> bool {
```
##########
datafusion/optimizer/src/analyzer/subquery.rs:
##########
@@ -233,13 +233,6 @@ fn check_inner_plan(
}
}
-fn contains_outer_reference(inner_plan: &LogicalPlan) -> bool {
- inner_plan
- .expressions()
Review Comment:
❤️ for removing this copy 🚀
##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -91,7 +91,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
_ => Ok(Transformed::no(plan)),
}
}
- _ if plan.expressions().iter().any(|expr| expr.contains_outer())
=> {
+ _ if plan.contains_outer_reference() => {
Review Comment:
And another copy gone!
##########
datafusion/common/src/tree_node.rs:
##########
@@ -292,6 +292,20 @@ pub trait TreeNode: Sized {
)
}
+ fn exists<F: FnMut(&Self) -> bool>(&self, mut f: F) -> bool {
Review Comment:
Can we please add some documentation to this method?
Perhaps something like
```suggestion
/// Returns true if `f` returns true for node in the tree.
///
/// Stops recursion as soon as a matching node is found
fn exists<F: FnMut(&Self) -> bool>(&self, mut f: F) -> bool {
```
--
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]
