This is an automated email from the ASF dual-hosted git repository. mbudiu pushed a commit to branch issue7070 in repository https://gitbox.apache.org/repos/asf/calcite.git
commit c3cecfcedfa1a991202d6da719c324564762685f Author: Mihai Budiu <[email protected]> AuthorDate: Mon Jul 14 15:09:02 2025 -0700 [CALCITE-7070] FILTER_REDUCE_EXPRESSIONS crashes on expression BETWEEN ( NULL) AND X Signed-off-by: Mihai Budiu <[email protected]> --- .../apache/calcite/plan/RelOptPredicateList.java | 34 +++++++++++++++++++++- .../calcite/rel/metadata/RelMdPredicates.java | 15 +++++++--- .../java/org/apache/calcite/rex/RexSimplify.java | 30 ++++++++++++++++++- .../main/java/org/apache/calcite/rex/RexUtil.java | 6 ++-- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java b/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java index fa1236a31e..1f8a846160 100644 --- a/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java +++ b/core/src/main/java/org/apache/calcite/plan/RelOptPredicateList.java @@ -18,9 +18,11 @@ import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.rex.RexNode; import org.apache.calcite.rex.RexUtil; import org.apache.calcite.sql.SqlKind; +import org.apache.calcite.util.Litmus; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -45,7 +47,7 @@ * expression that has a predicate {@code y < 10} then the pulled up predicates * for the Filter are {@code [y < 10, x > 1]}. * - * <p><b>Inferred predicates</b> only apply to joins. If there there is a + * <p><b>Inferred predicates</b> only apply to joins. If there is a * predicate on the left input to a join, and that predicate is over columns * used in the join condition, then a predicate can be inferred on the right * input to the join. (And vice versa.) @@ -104,6 +106,36 @@ private RelOptPredicateList(ImmutableList<RexNode> pulledUpPredicates, this.rightInferredPredicates = requireNonNull(rightInferredPredicates, "rightInferredPredicates"); this.constantMap = requireNonNull(constantMap, "constantMap"); + + // Validate invariants required + // (unfortunately the style rules don't allow us to move this to a separate function). + // Do not allow comparisons with null literals in pulledUpPredicates + for (RexNode predicate : this.pulledUpPredicates) { + switch (predicate.getKind()) { + case EQUALS: + case NOT_EQUALS: + case LESS_THAN: + case LESS_THAN_OR_EQUAL: + case GREATER_THAN: + case GREATER_THAN_OR_EQUAL: + final RexCall call = (RexCall) predicate; + final RexNode left = call.getOperands().get(0); + if (left.getKind() == SqlKind.LITERAL) { + RexLiteral literal = (RexLiteral) left; + Litmus.THROW.check(literal.getValue() != null, + "Comparison with NULL in pulledUpPredicates"); + } + final RexNode right = call.getOperands().get(1); + if (right.getKind() == SqlKind.LITERAL) { + RexLiteral literal = (RexLiteral) right; + Litmus.THROW.check(literal.getValue() != null, + "Comparison with NULL in pulledUpPredicates"); + } + break; + default: + break; + } + } } /** Creates a RelOptPredicateList with only pulled-up predicates, no inferred diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java index 9dcfb8e92e..3577b487bb 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPredicates.java @@ -318,12 +318,19 @@ public RelOptPredicateList getPredicates(Filter filter, RelMetadataQuery mq) { final RelNode input = filter.getInput(); final RexBuilder rexBuilder = filter.getCluster().getRexBuilder(); final RelOptPredicateList inputInfo = mq.getPulledUpPredicates(input); - + List<RexNode> predicates = + RexUtil.retainDeterministic(RelOptUtil.conjunctions(filter.getCondition())); + RelOptCluster cluster = filter.getCluster(); + final RexExecutor executor = + Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR); + RexSimplify simplify = new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, executor); + // The RelOptPredicateList data structure requires this invariant: no comparisons with null + List<RexNode> simplified = predicates.stream() + .map(e -> simplify.simplifyComparisonWithNull(e, RexUnknownAs.FALSE)) + .collect(Collectors.toList()); return Util.first(inputInfo, RelOptPredicateList.EMPTY) .union(rexBuilder, - RelOptPredicateList.of(rexBuilder, - RexUtil.retainDeterministic( - RelOptUtil.conjunctions(filter.getCondition())))); + RelOptPredicateList.of(rexBuilder, simplified)); } /** diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java index 86345533e3..d1b6ae5d9a 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -757,6 +757,11 @@ private <C extends Comparable<C>> RexNode simplifyComparison(RexCall e, } } + RexNode node = simplifyComparisonWithNull(e, unknownAs); + if (node instanceof RexLiteral) { + return node; + } + // If none of the arguments were simplified, return the call unchanged. final RexNode e2; if (operands.equals(e.operands)) { @@ -767,6 +772,28 @@ private <C extends Comparable<C>> RexNode simplifyComparison(RexCall e, return simplifyUsingPredicates(e2, clazz); } + /** + * If this RexNode is a comparison against NULL, return a simplified form, + * otherwise return it unchanged. + */ + public RexNode simplifyComparisonWithNull(RexNode e, RexUnknownAs unknownAs) { + final Comparison comparison = Comparison.of(e); + if (comparison != null) { + boolean againstNull = comparison.literal.isNull(); + // There is another possibility to check: in a comparison like 1 = null, + // the "non-literal" side of the Comparison can be null + if (comparison.ref instanceof RexLiteral) { + againstNull = againstNull || ((RexLiteral) comparison.ref).isNull(); + } + if (againstNull) { + return unknownAs == FALSE + ? rexBuilder.makeLiteral(false) + : rexBuilder.makeNullLiteral(e.getType()); + } + } + return e; + } + /** * Simplifies a conjunction of boolean expressions. */ @@ -2081,7 +2108,8 @@ private static <C extends Comparable<C>> RangeSet<C> residue(RexNode ref, if (comparison != null && comparison.ref.equals(ref)) { final C c1 = comparison.literal.getValueAs(clazz); if (c1 == null) { - continue; + throw new AssertionError("value must not be null in " + + comparison.literal); } switch (predicate.getKind()) { case NOT_EQUALS: diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java index d4cc04c6eb..4b134a1439 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java +++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java @@ -819,13 +819,13 @@ public static boolean isDeterministic(RexNode e) { } public static List<RexNode> retainDeterministic(List<RexNode> list) { - List<RexNode> conjuctions = new ArrayList<>(); + List<RexNode> conjunctions = new ArrayList<>(); for (RexNode x : list) { if (isDeterministic(x)) { - conjuctions.add(x); + conjunctions.add(x); } } - return conjuctions; + return conjunctions; } /**
