This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 30bfe156a2 [common] Never compare null literals when merging range
predicates (#9647)
30bfe156a2 is described below
commit 30bfe156a2dc85a0cf8da79f6ea393383e04dfb8
Author: YangJie <[email protected]>
AuthorDate: Fri Sep 11 03:09:36 2026 -0400
[common] Never compare null literals when merging range predicates (#9647)
---
.../java/org/apache/paimon/predicate/Between.java | 32 ++++++++++++++++--
.../org/apache/paimon/predicate/BetweenTest.java | 39 ++++++++++++++++++++++
2 files changed, 68 insertions(+), 3 deletions(-)
diff --git
a/paimon-common/src/main/java/org/apache/paimon/predicate/Between.java
b/paimon-common/src/main/java/org/apache/paimon/predicate/Between.java
index 30d317b224..dce11132c6 100644
--- a/paimon-common/src/main/java/org/apache/paimon/predicate/Between.java
+++ b/paimon-common/src/main/java/org/apache/paimon/predicate/Between.java
@@ -27,6 +27,7 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import static org.apache.paimon.predicate.CompareUtils.compareLiteral;
@@ -133,7 +134,7 @@ public class Between extends LeafTernaryFunction {
if (leafPredicate.function() == LessOrEqual.INSTANCE) {
if (lessOrEqual == null) {
lessOrEqual = leafPredicate;
- } else {
+ } else if (!hasNullLiteral(lessOrEqual) &&
!hasNullLiteral(leafPredicate)) {
lessOrEqual =
compareLiteral(
type,
@@ -142,11 +143,14 @@ public class Between extends LeafTernaryFunction {
< 0
? lessOrEqual
: leafPredicate;
+ } else {
+ // A null bound matches nothing; keep the null-bearing
predicate.
+ lessOrEqual = hasNullLiteral(lessOrEqual) ? lessOrEqual :
leafPredicate;
}
} else if (leafPredicate.function() == GreaterOrEqual.INSTANCE) {
if (greaterOrEqual == null) {
greaterOrEqual = leafPredicate;
- } else {
+ } else if (!hasNullLiteral(greaterOrEqual) &&
!hasNullLiteral(leafPredicate)) {
greaterOrEqual =
compareLiteral(
type,
@@ -155,6 +159,9 @@ public class Between extends LeafTernaryFunction {
> 0
? greaterOrEqual
: leafPredicate;
+ } else {
+ greaterOrEqual =
+ hasNullLiteral(greaterOrEqual) ? greaterOrEqual :
leafPredicate;
}
} else {
result.add(leafPredicate);
@@ -166,7 +173,12 @@ public class Between extends LeafTernaryFunction {
// Determine which is the lower bound and which is the upper bound
Object lowerBound = greaterOrEqual.literals().get(0);
Object upperBound = lessOrEqual.literals().get(0);
- if (compareLiteral(type, lowerBound, upperBound) >= 0) {
+ if (lowerBound == null || upperBound == null) {
+ // A null bound makes the conjunction match nothing; never
compare
+ // nulls (SQL null literals are unordered).
+ result.add(lessOrEqual);
+ result.add(greaterOrEqual);
+ } else if (compareLiteral(type, lowerBound, upperBound) >= 0) {
// No valid intersection, keep all original predicates
result.add(lessOrEqual);
result.add(greaterOrEqual);
@@ -190,6 +202,10 @@ public class Between extends LeafTernaryFunction {
return result;
}
+ private static boolean hasNullLiteral(LeafPredicate predicate) {
+ return predicate.literals().stream().anyMatch(Objects::isNull);
+ }
+
private static List<LeafPredicate> mergeMultipleBetweens(
FieldTransform field, List<LeafPredicate> predicates) {
List<LeafPredicate> results = new ArrayList<>();
@@ -211,10 +227,15 @@ public class Between extends LeafTernaryFunction {
Object maxLower = null;
Object minUpper = null;
+ boolean anyNullLiteral = false;
for (LeafPredicate between : betweens) {
Object lower = between.literals().get(0);
Object upper = between.literals().get(1);
+ if (lower == null || upper == null) {
+ anyNullLiteral = true;
+ continue;
+ }
if (maxLower == null || compareLiteral(fieldType, lower, maxLower)
> 0) {
maxLower = lower;
}
@@ -222,6 +243,11 @@ public class Between extends LeafTernaryFunction {
minUpper = upper;
}
}
+ if (anyNullLiteral) {
+ // A null bound makes the conjunction match nothing; leave the
predicates
+ // unmerged instead of comparing nulls.
+ return predicates;
+ }
// Check if intersection is valid
if (maxLower != null
diff --git
a/paimon-common/src/test/java/org/apache/paimon/predicate/BetweenTest.java
b/paimon-common/src/test/java/org/apache/paimon/predicate/BetweenTest.java
index f5692d85f5..20e3966ab2 100644
--- a/paimon-common/src/test/java/org/apache/paimon/predicate/BetweenTest.java
+++ b/paimon-common/src/test/java/org/apache/paimon/predicate/BetweenTest.java
@@ -18,6 +18,7 @@
package org.apache.paimon.predicate;
+import org.apache.paimon.data.GenericRow;
import org.apache.paimon.types.IntType;
import org.apache.paimon.types.RowType;
@@ -47,6 +48,44 @@ class BetweenTest {
assertThat(compoundResult.children().get(0)).isEqualTo(isNotNull);
}
+ @Test
+ public void testNullLiteralBoundsDoNotCrash() {
+ PredicateBuilder builder = new PredicateBuilder(RowType.of(new
IntType()));
+ // x <= NULL AND x >= 1: Flink pushdown keeps null literals; optimize()
+ // previously crashed comparing null (Unsupported type / NPE).
+ Predicate lteNull = builder.lessOrEqual(0, null);
+ Predicate gte = builder.greaterOrEqual(0, 1);
+ Predicate and = PredicateBuilder.and(Arrays.asList(lteNull, gte));
+ assertThat(and).isNotNull();
+
+ // Two <= bounds where one is null: keeps the null-bearing predicate.
+ Predicate lte10 = builder.lessOrEqual(0, 10);
+ Predicate andNulls = PredicateBuilder.and(Arrays.asList(lteNull,
lte10, gte));
+ assertThat(andNulls).isNotNull();
+
+ // Two >= bounds where one is null.
+ Predicate gteNull = builder.greaterOrEqual(0, null);
+ Predicate andGteNulls =
+ PredicateBuilder.and(Arrays.asList(gteNull,
builder.greaterOrEqual(0, 5), lte10));
+ assertThat(andGteNulls).isNotNull();
+
+ // Two BETWEENs where one has a null bound stay unmerged: dropping the
+ // null-bound BETWEEN would wrongly match rows 1..4.
+ Predicate betweenNull = builder.between(0, null, 5);
+ Predicate between = builder.between(0, 1, 4);
+ Predicate andBetweens =
PredicateBuilder.and(Arrays.asList(betweenNull, between));
+ assertThat(andBetweens).isInstanceOf(CompoundPredicate.class);
+ CompoundPredicate compound = (CompoundPredicate) andBetweens;
+ assertThat(compound.function()).isInstanceOf(And.class);
+ assertThat(compound.children()).hasSize(2);
+
+ // The null-bearing predicate retained by the merge still evaluates to
false.
+ Predicate merged = PredicateBuilder.and(Arrays.asList(lteNull, gte));
+ GenericRow row = new GenericRow(1);
+ row.setField(0, 3);
+ assertThat(merged.test(row)).isFalse();
+ }
+
@Test
public void testTryRewriteBetweenPredicateBasic() {
// Test basic case: AND(a>=1, a<=10, a is not null) should be
rewritten to BETWEEN
