my-vegetable-has-exploded commented on code in PR #8654:
URL: https://github.com/apache/arrow-datafusion/pull/8654#discussion_r1436401368
##########
datafusion/physical-expr/src/utils/guarantee.rs:
##########
@@ -168,14 +195,21 @@ impl LiteralGuarantee {
// if all terms are 'col <op> literal' with the same column
// and operation we can infer any guarantees
+ //
+ // For those like (a != bar OR a != baz).
+ // We can't combine the (a != bar OR a != baz) part, but
+ // it also doesn't invalidate our knowledge that a !=
+ // foo is required for the expression to be true.
+ // So we can only create a multi guarantee for `=`
+ // (or a single value). (e.g. ignore `a != foo OR a !=
bar`)
let first_term = &terms[0];
if terms.iter().all(|term| {
term.col.name() == first_term.col.name()
- && term.op == first_term.op
+ && term.guarantee == Guarantee::In
Review Comment:
`a != foo OR a != bar` would be filtered out here, So we don't need to check
`new_values.len() == 1` in `aggregate_multi_conjunct`. If not,
`new_values.len() == 1` will confilct with the LiteralGuarantee result from
`InList` who may have multi literals. And I think refactored code is easier to
follow 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
