Add additional expr boolean simplification rules#4200
Conversation
| op: Or, | ||
| right, | ||
| }) if is_false(&right) => *left, | ||
| // A OR !A ---> true (if A not nullable) |
There was a problem hiding this 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
There was a problem hiding this comment.
👍 I suspect we would need to add some logic to https://github.com/apache/arrow-datafusion/blob/master/datafusion/optimizer/src/type_coercion.rs
| // 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")); |
There was a problem hiding this comment.
Without this change, these test fail.
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @Jefffrey and @Ted-Jiang
|
Benchmark runs are scheduled for baseline = d2814c9 and contender = 9de893e. 9de893e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
4 participants
Which issue does this PR close?
Closes #1406.
Rationale for this change
What changes are included in this PR?
Introduce new boolean simplification rules for expressions:
A AND !Aevaluates tofalsegivenAis not nullableA OR !Aevaluates totruegivenAis not nullableAre these changes tested?
Added unit tests
Are there any user-facing changes?