You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/09/05 08:50:40 UTC

[PR] [common] Never compare null literals when merging range predicates [paimon]

LuciferYang opened a new pull request, #9647:
URL: https://github.com/apache/paimon/pull/9647

   ### Purpose
   
   close #9646
   
   `Between.optimize` merges `<=` and `>=` predicates on one field into a `BETWEEN`, and it ordered the two bounds without checking them:
   
   ```java
   Object lowerBound = greaterOrEqual.literals().get(0);
   Object upperBound = lessOrEqual.literals().get(0);
   if (compareLiteral(type, lowerBound, upperBound) >= 0) {
   ```
   
   With a null bound that is a `NullPointerException` from `compareTo(null)`, or `RuntimeException("Unsupported type")` when the null is the first argument. `CALL sys.compact(table => 'db.t', where => 'dt >= 1 AND dt <= null')` hits it: procedure filters go through `ExpressionHelper.resolveFilter`, which runs `ConstantFolding` but not `NullPropagation`, so the null literal reaches Paimon intact.
   
   A null bound has no order, so the pair is now left unmerged and evaluated by the rules that already handle this: `LeafBinaryFunction.test` treats a null literal as no match. Merging into a `BETWEEN` is an optimization, and skipping it changes nothing semantically.
   
   `compareLiteral` also gained a guard, since it is the function whose contract was implicit: a null literal now gets an `IllegalArgumentException` naming the type instead of a `NullPointerException` from inside `compareTo`. Its other callers (`NotIn`, `NotBetween`, `LessThan`) either null-check first or are only reached with non-null bounds, so nothing else changes.
   
   I did consider short-circuiting the merge to `alwaysFalse()`, since `k >= 1 AND k <= NULL` really does match nothing. I left it out: `optimize`'s job is merging predicates, and having it start producing `alwaysFalse` reaches past what `PredicateBuilder.and` asked for, while the evaluation layer already answers false for these.
   
   ### Tests
   
   `BetweenTest.testNullLiteralBoundsDoNotCrash` builds `k >= 1 AND k <= null` through `PredicateBuilder.and`, asserts it does not throw, and asserts the merged predicate evaluates to false for a row. It also covers a null lower bound and two `BETWEEN`s where one carries a null.
   
   Against the unfixed optimizer the test errors with a `NullPointerException`.
   
   `mvn -pl paimon-common -Dtest=BetweenTest,PredicateTest test` on JDK 8: 50 tests, 0 failures. `spotless:check` and `checkstyle:check` on paimon-common are clean.
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [common] Never compare null literals when merging range predicates [paimon]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #9647:
URL: https://github.com/apache/paimon/pull/9647#issuecomment-5631425629

   Thank you @JingsongLi 


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [common] Never compare null literals when merging range predicates [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9647:
URL: https://github.com/apache/paimon/pull/9647#discussion_r3976329923


##########
paimon-common/src/main/java/org/apache/paimon/predicate/CompareUtils.java:
##########
@@ -27,6 +27,11 @@ public class CompareUtils {
     private CompareUtils() {}
 
     public static int compareLiteral(DataType type, Object v1, Object v2) {
+        // SQL null literals are unordered, so a caller that merges range predicates has to

Review Comment:
   Please do not modify this.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [common] Never compare null literals when merging range predicates [paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #9647:
URL: https://github.com/apache/paimon/pull/9647


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org