alamb commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1408369508
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1917,6 +1923,84 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ fn is_scalar(&self) -> bool {
Review Comment:
Can we please add docstrings that explains what this function does / under
what circumstances it returns true?
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1917,6 +1923,84 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ fn is_scalar(&self) -> bool {
+ let schema = self.input.schema();
+
+ let functional_dependencies =
self.input.schema().functional_dependencies();
+ let unique_keys = functional_dependencies.iter().filter(|dep| {
+ let nullable = dep.nullable
+ && dep
+ .source_indices
+ .iter()
+ .any(|&source| schema.field(source).is_nullable());
+ !nullable
+ && dep.mode == Dependency::Single
+ && dep.target_indices.len() == schema.fields().len()
+ });
+
+ /// Splits a conjunctive [`Expr`] such as `A AND B AND C` => `[A, B,
C]`
+ pub fn split_conjunction(expr: &Expr) -> Vec<&Expr> {
+ split_conjunction_impl(expr, vec![])
+ }
+
+ fn split_conjunction_impl<'a>(
+ expr: &'a Expr,
+ mut exprs: Vec<&'a Expr>,
+ ) -> Vec<&'a Expr> {
+ match expr {
+ Expr::BinaryExpr(BinaryExpr {
+ right,
+ op: Operator::And,
+ left,
+ }) => {
+ let exprs = split_conjunction_impl(left, exprs);
+ split_conjunction_impl(right, exprs)
+ }
+ Expr::Alias(Alias { expr, .. }) =>
split_conjunction_impl(expr, exprs),
+ other => {
+ exprs.push(other);
+ exprs
+ }
+ }
+ }
+
+ let exprs = split_conjunction(&self.predicate);
+ let unique_cols: HashSet<_> = exprs
Review Comment:
this is really a list of columns that appear in equality predicates, so
perhaps calling it something like
```suggestion
let eq_pred_cols: HashSet<_> = exprs
```
Would make the intent clear
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1917,6 +1923,84 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ fn is_scalar(&self) -> bool {
+ let schema = self.input.schema();
+
+ let functional_dependencies =
self.input.schema().functional_dependencies();
+ let unique_keys = functional_dependencies.iter().filter(|dep| {
+ let nullable = dep.nullable
+ && dep
+ .source_indices
+ .iter()
+ .any(|&source| schema.field(source).is_nullable());
+ !nullable
+ && dep.mode == Dependency::Single
+ && dep.target_indices.len() == schema.fields().len()
+ });
+
+ /// Splits a conjunctive [`Expr`] such as `A AND B AND C` => `[A, B,
C]`
+ pub fn split_conjunction(expr: &Expr) -> Vec<&Expr> {
Review Comment:
I believe this is a duplicate of
https://docs.rs/datafusion/latest/datafusion/optimizer/utils/fn.split_conjunction.html
Perhaps we could move the function into
https://github.com/apache/arrow-datafusion/blob/main/datafusion/expr/src/utils.rs
instead so we can avoid the copy/paste?
--
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]
