You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Jesse-Bakker (via GitHub)" <gi...@apache.org> on 2023/11/23 09:18:04 UTC
[PR] Detect when filters make subqueries scalar [arrow-datafusion]
Jesse-Bakker opened a new pull request, #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312
## Which issue does this PR close?
Part of #3725
## Rationale for this change
In some cases, it is possible to prove that a Filter only ever produces one
row. In those cases, such a filter may be used in a scalar subquery to ensure
it is in fact scalar, allowing for more flexible use of scalar subqueries.
## What changes are included in this PR?
This adds an `is_scalar()` method to `Filter`, which will check if there is a
unique functional dependence which is covered by the `Filter`'s predicate.
This is used in `LogicalPlan::max_rows()` to provide a tighter bound on the
maximum number of rows returned in the presence of `Filter`s.
## Are these changes tested?
This is directly tested with a unit test and with a `sqllogictest` that exercises
the more flexible use of scalar subqueries.
## Are there any user-facing changes?
Scalar subqueries are more flexible. Previous constraints were not documented
as far as I can tell
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "Jesse-Bakker (via GitHub)" <gi...@apache.org>.
Jesse-Bakker commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1413658701
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ fn is_scalar(&self) -> bool {
Review Comment:
Do I understand correctly that the suggestion is to rename this method to `Filter::is_uniform()`?
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "Jesse-Bakker (via GitHub)" <gi...@apache.org>.
Jesse-Bakker commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1413659204
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ 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()
+ });
+
+ let exprs = split_conjunction(&self.predicate);
+ let eq_pred_cols: HashSet<_> = exprs
+ .iter()
+ .filter_map(|expr| {
+ let Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: Operator::Eq,
+ right,
+ }) = expr
+ else {
+ return None;
+ };
+ // This is a no-op filter expression
+ if left == right {
+ return None;
+ }
+
+ match (left.as_ref(), right.as_ref()) {
+ (Expr::Column(_), Expr::Column(_)) => None,
+ (Expr::Column(c), _) | (_, Expr::Column(c)) => {
+ Some(schema.index_of_column(c).unwrap())
Review Comment:
Should it be `schema.index_of_column(c).ok()` instead?
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "Jesse-Bakker (via GitHub)" <gi...@apache.org>.
Jesse-Bakker commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1408951755
##########
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:
There are quite a few functions in `datafusion-optimizer::utils` which only depend on `LogicalPlan` or `Expr` and are good candidates to move to `datafusion-expr::utils`. I'll file a follow-up PR for that.
EDIT: #8354
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1413698745
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ fn is_scalar(&self) -> bool {
Review Comment:
Yes, and add these definition into doc/
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#issuecomment-1828758686
Thank you @Jesse-Bakker -- I plan to review this carefully tomorrow
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#issuecomment-1833077215
`unique constraints` and `function dependences` is complex feature, I prepare to think more about it.
I will review this PR and think more about it. If this PR isn't urgent, please wait for me.
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#issuecomment-1843796123
Thanks again @Jesse-Bakker and @jackwener
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "Jesse-Bakker (via GitHub)" <gi...@apache.org>.
Jesse-Bakker commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1408951755
##########
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:
There are quite a few functions in `datafusion-optimizer::utils` which only depend on `LogicalPlan` or `Expr` and are good candidates to move to `datafusion-expr::utils`. I'll file a follow-up PR for that.
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1412757521
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ fn is_scalar(&self) -> bool {
Review Comment:
I suggest use `Uniform slot` and `Unique slot` to describe `FDs` instead of `scalar`.
Commonly used Functional dependencies : including
`uniform slot`, which means `ndv <= 1`
`unique slot`, which means `ndv = row`
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ 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()
+ });
+
+ let exprs = split_conjunction(&self.predicate);
+ let eq_pred_cols: HashSet<_> = exprs
+ .iter()
+ .filter_map(|expr| {
+ let Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: Operator::Eq,
+ right,
+ }) = expr
+ else {
+ return None;
+ };
+ // This is a no-op filter expression
+ if left == right {
+ return None;
+ }
+
+ match (left.as_ref(), right.as_ref()) {
+ (Expr::Column(_), Expr::Column(_)) => None,
+ (Expr::Column(c), _) | (_, Expr::Column(c)) => {
+ Some(schema.index_of_column(c).unwrap())
Review Comment:
```suggestion
schema.index_of_column(c).unwrap_err()
```
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "Jesse-Bakker (via GitHub)" <gi...@apache.org>.
Jesse-Bakker commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1409720710
##########
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'll rebase tomorrow to get rid of this TODO
--
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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#issuecomment-1830660769
The only thing I think is strictly needed to approve this PR is docstrings for "is_scalar" -- the rest is "nice to have" from my perspective / could be done as a follow on 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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#issuecomment-1843757171
I took the liberty of merging up from main to resolve a conflict in this 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
Re: [PR] Detect when filters on unique constraints make subqueries scalar [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #8312:
URL: https://github.com/apache/arrow-datafusion/pull/8312#discussion_r1416292022
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1935,6 +1942,73 @@ impl Filter {
Ok(Self { predicate, input })
}
+
+ /// Is this filter guaranteed to return 0 or 1 row in a given instantiation?
+ ///
+ /// This function will return `true` if its predicate contains a conjunction of
+ /// `col(a) = <expr>`, where its schema has a unique filter that is covered
+ /// by this conjunction.
+ ///
+ /// For example, for the table:
+ /// ```sql
+ /// CREATE TABLE t (a INTEGER PRIMARY KEY, b INTEGER);
+ /// ```
+ /// `Filter(a = 2).is_scalar() == true`
+ /// , whereas
+ /// `Filter(b = 2).is_scalar() == false`
+ /// and
+ /// `Filter(a = 2 OR b = 2).is_scalar() == false`
+ 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()
+ });
+
+ let exprs = split_conjunction(&self.predicate);
+ let eq_pred_cols: HashSet<_> = exprs
+ .iter()
+ .filter_map(|expr| {
+ let Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: Operator::Eq,
+ right,
+ }) = expr
+ else {
+ return None;
+ };
+ // This is a no-op filter expression
+ if left == right {
+ return None;
+ }
+
+ match (left.as_ref(), right.as_ref()) {
+ (Expr::Column(_), Expr::Column(_)) => None,
+ (Expr::Column(c), _) | (_, Expr::Column(c)) => {
+ Some(schema.index_of_column(c).unwrap())
Review Comment:
I don't understand the suggestion @jackwener the code here basically panic's if the column reference isn't in the schema, which seems like a reasonable thing to do
I suppose ignoring the error (which is what `schema.index_of_column(c).ok()` would do) is also ok, though I personally think the code as it exists in this PR is better as it will fail fast on a really bad plan rather than silently ignore 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org