Ted-Jiang commented on code in PR #4200:
URL: https://github.com/apache/arrow-datafusion/pull/4200#discussion_r1021374351
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -470,6 +470,22 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
op: Or,
right,
}) if is_false(&right) => *left,
+ // A OR !A ---> true (if A not nullable)
Review Comment:
Nice! It's quit complicated deal with `null`.
in datafsuion:
```
❯ select true or NULL ;
Plan("'Boolean OR Null' can't be evaluated because there isn't a common type
to coerce the types to")
❯ select true or FALSE ;
+---------------------------------+
| Boolean(true) OR Boolean(false) |
+---------------------------------+
| true |
+---------------------------------+
1 row in set. Query took 0.003 seconds.
❯
```
RUN in spark-sql
```
spark-sql> select true or false;
true
Time taken: 3.139 seconds, Fetched 1 row(s)
spark-sql> select true or NULL;
true
Time taken: 0.076 seconds, Fetched 1 row(s)
spark-sql> select FALSE or NULL;
NULL
Time taken: 0.06 seconds, Fetched 1 row(s)
spark-sql>
```
I think its ok get error when run `NULL or xxx`
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -1105,6 +1149,18 @@ mod tests {
assert_eq!(simplify(expr_b), expected);
}
+ #[test]
+ fn test_simplify_and_not_self() {
+ // A AND !A if A is not nullable --> false
+ // !A AND A if A is not nullable --> false
+ let expr_a = col("c2_non_null").and(col("c2_non_null").not());
+ let expr_b = col("c2_non_null").not().and(col("c2_non_null"));
Review Comment:
tests win! 👍
--
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]
