[CALCITE-7635] Simplification result of conjunction of comparisons depends on terms order#5070
[CALCITE-7635] Simplification result of conjunction of comparisons depends on terms order#5070rubenada wants to merge 2 commits into
Conversation
…pends on terms order
| // Range is always satisfied given these predicates; but nullability might | ||
| // be problematic | ||
| if (unknownAs != UNKNOWN) { | ||
| // If unknownAs FALSE: row is already excluded for null input, so the IS_NOT_NULL |
There was a problem hiding this comment.
do your tests cover all 3 cases here?
There was a problem hiding this comment.
added new test covering 3 cases
| // E.g. given predicate "x >= 5" and term "x between 3 and 10" | ||
| // we weaken to term to "x between 5 and 10". | ||
| final RexNode term2 = simplifyUsingPredicates(term, clazz); | ||
| final RexNode term2 = simplifyUsingPredicates(term, clazz, FALSE); |
There was a problem hiding this comment.
Can you argue why FALSE is always correct here?
There was a problem hiding this comment.
This method is called simplifyAnd2ForUnknownAsFalse, and the javadoc (on the overloaded method) states "... we assume that if the expression returns UNKNOWN it will be interpreted as FALSE". There are other cases in this method were we need to call another one passing a RexUnknownAs parameter, and FALSE value is used (e.g. return simplify(terms.get(0), FALSE) in line 1873)
|
2 participants
Jira Link
CALCITE-7635
Changes Proposed
Return IS NOT NULL in RexSimplify#simplifyUsingPredicates (when ranges are satisfied considered previous predicates) only if unknownAs is UNKNOWN (otherwise simply return TRUE).
See more details in Jira.