Skip to content

[fix](rbo) Preserve semantics in predicate inference #67919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
feiniaofeiafei wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[fix](rbo) Preserve semantics in predicate inference #67919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38a5034
[fix](fe) Preserve inequality relations when retaining input predicates
feiniaofeiafei Sep 13, 2026
1acbe05
[fix](fe) Restrict predicate substitution to value-preserving contexts
feiniaofeiafei Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferPredicateByReplace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.types.DecimalV2Type;
import org.apache.doris.nereids.types.DecimalV3Type;
import org.apache.doris.nereids.util.ExpressionUtils;
Expand Down Expand Up @@ -153,12 +154,15 @@ private static <T extends Expression> Set<Expression> getEqualSetAndDoReplace(T
ExpressionAnalyzer analyzer = new ReplaceAnalyzer(null, new Scope(ImmutableList.of()), null, false, false);
Set<Expression> res = new LinkedHashSet<>();
for (T equals : equalSet) {
Map<Expression, Expression> replaceMap = new HashMap<>();
replaceMap.put(equals, replaceToThis);
if (!exprPredicates.containsKey(equals)) {
continue;
}
Map<Expression, Expression> replaceMap = new HashMap<>();
replaceMap.put(equals, replaceToThis);
for (Expression predicate : exprPredicates.get(equals)) {
if (!canReplace(equals, replaceToThis, predicate)) {
continue;
}
Expression newPredicates = ExpressionUtils.replace(predicate, replaceMap);
try {
Expression analyzed = analyzer.analyze(newPredicates);
Expand All @@ -171,6 +175,27 @@ private static <T extends Expression> Set<Expression> getEqualSetAndDoReplace(T
return res;
}

private static boolean canReplace(Expression source, Expression target, Expression predicate) {
Expression comparison = predicate instanceof Not ? predicate.child(0) : predicate;
// Direct comparisons observe comparison equality rather than a value's type or representation.
// Do not descend through functions, casts or OR to apply this exception.
if ((comparison instanceof ComparisonPredicate || comparison instanceof InPredicate)

@github-actions github-actions Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not propagate through lossy TIMESTAMPTZ casts

This direct exception assumes the equality pair was extracted through injective casts, but PredicateInferUtils.validForInfer currently peels any TIMESTAMPTZ -> DATETIMEV2 cast even though TimeStampTzType.isInjectiveCastTo rejects that conversion. In America/New_York fall-back, the reduced plan InnerJoin(CAST(l.tz AS DATETIMEV2(0)) = r.dt) can join l.tz=06:30Z to r.dt=01:30; a left filter NOT(l.tz = 05:30Z) is true, yet replacement produces NOT(r.dt = 01:30) and pushes false to the right child because both UTC instants cast to local 01:30. Scale reduction has the same problem (.1236 rounds to .124). Please stop exposing a raw equality through this cast unless childType.isInjectiveCastTo(targetType) holds, and cover the production join path; guarding only this branch would leave the same extracted pair available to inequality inference.

&& comparison.child(0).equals(source)) {
return true;
}
DataType type = source.getDataType();
// Comparison equality across types does not preserve type-sensitive expressions such as CAST to STRING.
if (!type.equals(target.getDataType())) {
return false;
}
// Only substitute types whose equality preserves the value observed by enclosing expressions.
// In particular, FLOAT/DOUBLE equality cannot distinguish signed zero, but SIGNBIT can.
// Comparisons can still be propagated separately by UnequalPredicateInfer.
return type.isBooleanType() || type.isIntegralType() || type.isDecimalLikeType()

@github-actions github-actions Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Exclude NoneMovableFunction from replacement inference

This allowlist still treats every deterministic expression over equal same-type values as movable, but assert_true is deterministic and implements NoneMovableFunction. For the reduced plan Join(a=b) with left child Filter(assert_true(a > 0, 'bad') OR a > 10) -> Scan L(a={1}) and right child Scan R(b={-1,1}), visitOr maps the sole slot a to the whole OR and this branch infers assert_true(b > 0, 'bad') OR b > 10. InferPredicates.inferNewPredicate installs that filter on the right child, so the unmatched b=-1 row raises even though the original plan returns the 1=1 match. Please reject predicates containing NoneMovableFunction (as the other movement rules do) before cloning them across an equality.

|| type.isStringLikeType() || type.isIPType()
|| (type.isDateLikeType() && !type.isTimeStampTzType());
}

/* Extract the equivalence relationship a=b, and when case (d_tinyint as int)=d_int is encountered,
remove the cast and extract d_tinyint=d_int
EqualPairs is the output parameter and the equivalent pair of predicate derivation input,
Expand Down
3 changes: 2 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/UnequalPredicateInfer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ public Set<Expression> chooseInputPredicates(Relation[][] chosen) {
clear(chosen, left, right, type);
} else if (deduced[left][right] != type) {
keep[i] = true;
set(deduced, left, right, Relation.EQ);
// Preserve the relation of the retained predicate; an inequality is not an equality.
set(deduced, left, right, type);

@github-actions github-actions Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve strict edges after recording the retained relation

This assignment creates a head-only weakening when equality and strict/non-strict edges overlap. For the ordered filter a = c AND c > a AND c >= b AND b > c AND b = c, selection starts with a=b EQ, a=c EQ, and b>a GT. Here c>=b enters this branch. Recording it as GTE makes the closure treat the later b>c input as redundant; generation then emits a=b and clear(..., EQ) also removes the distinct reverse b>a GT. The result keeps only equalities plus c>=b, so a=b=c passes although the input is contradictory. With the base revision's EQ write, b>c is retained and the contradiction remains. Please preserve simultaneous reverse relations (or make EQ clearing relation-aware) and add this ordered case plus permutations to the semantic oracle and a filter regression.

expandGraph(deduced, left, right);
if (type == Relation.EQ) {
expandGraph(deduced, right, left);
Expand Down
125 changes: 125 additions & 0 deletions ...ore/src/test/java/org/apache/doris/nereids/rules/rewrite/InferPredicateByReplaceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,50 @@
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Abs;
import org.apache.doris.nereids.trees.expressions.functions.scalar.DateTrunc;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Length;
import org.apache.doris.nereids.trees.expressions.functions.scalar.SignBit;
import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DateTimeLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal;
import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
import org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral;
import org.apache.doris.nereids.types.ArrayType;
import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.types.BooleanType;
import org.apache.doris.nereids.types.CharType;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.types.DateTimeType;
import org.apache.doris.nereids.types.DateTimeV2Type;
import org.apache.doris.nereids.types.DateType;
import org.apache.doris.nereids.types.DateV2Type;
import org.apache.doris.nereids.types.DecimalV3Type;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.types.IPv4Type;
import org.apache.doris.nereids.types.IPv6Type;
import org.apache.doris.nereids.types.IntegerType;
import org.apache.doris.nereids.types.StringType;
import org.apache.doris.nereids.types.TimeStampNsType;
import org.apache.doris.nereids.types.TinyIntType;
import org.apache.doris.nereids.types.VarcharType;
import org.apache.doris.nereids.util.PredicateInferUtils;
import org.apache.doris.nereids.util.TypeCoercionUtils;

import com.google.common.collect.ImmutableList;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;

public class InferPredicateByReplaceTest {
@Test
Expand Down Expand Up @@ -245,4 +268,106 @@ public void testNotInferWithTransitiveEqualitySameTable() {
Set<Expression> result = InferPredicateByReplace.infer(inputs);
Assertions.assertEquals(2, result.size());
}

static Stream<Arguments> replacementTypes() {
List<Arguments> cases = new ArrayList<>();
for (DataType type : ImmutableList.of(BooleanType.INSTANCE, IntegerType.INSTANCE, BigIntType.INSTANCE,
StringType.INSTANCE, DateV2Type.INSTANCE, DateTimeV2Type.of(0), DateTimeV2Type.of(6),
DecimalV3Type.createDecimalV3Type(9, 2), TimeStampNsType.INSTANCE,
CharType.createCharType(10), VarcharType.createVarcharType(10),
IPv4Type.INSTANCE, IPv6Type.INSTANCE)) {
cases.add(Arguments.of(type, type, true));
}
for (List<DataType> pair : ImmutableList.<List<DataType>>of(
ImmutableList.of(DateV2Type.INSTANCE, DateTimeV2Type.of(0)),
ImmutableList.of(DateTimeV2Type.of(0), DateTimeV2Type.of(6)),
ImmutableList.of(IntegerType.INSTANCE, BigIntType.INSTANCE),
ImmutableList.of(DecimalV3Type.createDecimalV3Type(9, 2),
DecimalV3Type.createDecimalV3Type(9, 3)))) {
cases.add(Arguments.of(pair.get(0), pair.get(1), false));
cases.add(Arguments.of(pair.get(1), pair.get(0), false));
}
cases.add(Arguments.of(FloatType.INSTANCE, FloatType.INSTANCE, false));
cases.add(Arguments.of(DoubleType.INSTANCE, DoubleType.INSTANCE, false));
cases.add(Arguments.of(ArrayType.of(DoubleType.INSTANCE), ArrayType.of(DoubleType.INSTANCE), false));
return cases.stream();
}

@ParameterizedTest(name = "{0} -> {1}, replace={2}")
@MethodSource("replacementTypes")
public void testTypeSensitiveReplacement(DataType sourceType, DataType targetType, boolean canReplace) {
SlotReference a = new SlotReference("a", sourceType);
SlotReference b = new SlotReference("b", targetType);
Expression equality = TypeCoercionUtils.processComparisonPredicate(new EqualTo(a, b));
Expression predicate = new EqualTo(new Length(new Cast(a, StringType.INSTANCE)), new IntegerLiteral(10));
Set<Expression> inputs = new LinkedHashSet<>(ImmutableList.of(equality, predicate));
Set<Expression> result = InferPredicateByReplace.infer(inputs);
if (canReplace) {
Expression expected = new EqualTo(new Length(new Cast(b, StringType.INSTANCE)), new IntegerLiteral(10));
Assertions.assertTrue(result.contains(expected), () -> "Missing " + expected + " in " + result);
} else {
Assertions.assertEquals(inputs, result);
}
}

@Test
public void testSignedZeroInOr() {
SlotReference x = new SlotReference("x", DoubleType.INSTANCE);
SlotReference y = new SlotReference("y", DoubleType.INSTANCE);
Expression predicate = new Or(new SignBit(x), new GreaterThan(x, new DoubleLiteral(1.0)));
Set<Expression> inputs = new LinkedHashSet<>(ImmutableList.of(new EqualTo(x, y), predicate));
// x = -0.0 and y = +0.0 satisfy the input, but not the predicate with x replaced by y.
Assertions.assertEquals(inputs, InferPredicateByReplace.infer(inputs));
Assertions.assertEquals(inputs, PredicateInferUtils.inferPredicate(inputs));
}

@Test
public void testSafeComparisonPropagation() {
SlotReference x = new SlotReference("x", DoubleType.INSTANCE, true, ImmutableList.of("left"));
SlotReference y = new SlotReference("y", DoubleType.INSTANCE, true, ImmutableList.of("right"));
Set<Expression> floating = new LinkedHashSet<>(ImmutableList.of(new EqualTo(x, y),
new GreaterThan(x, new DoubleLiteral(1.0))));
Assertions.assertTrue(PredicateInferUtils.inferPredicate(floating)
.contains(new GreaterThan(y, new DoubleLiteral(1.0))));

SlotReference small = new SlotReference("small", IntegerType.INSTANCE, true, ImmutableList.of("left"));
SlotReference wide = new SlotReference("wide", BigIntType.INSTANCE, true, ImmutableList.of("right"));
Set<Expression> integers = new LinkedHashSet<>(ImmutableList.of(
new EqualTo(new Cast(small, BigIntType.INSTANCE), wide),
new GreaterThan(small, new IntegerLiteral(1))));
Assertions.assertTrue(PredicateInferUtils.inferPredicate(integers).stream()
.anyMatch(p -> p instanceof GreaterThan && p.child(0).equals(wide)
&& p.child(1).equals(new BigIntLiteral(1))));
}

@Test
public void testDirectComparisonReplacement() {
SlotReference small = new SlotReference("small", IntegerType.INSTANCE);
SlotReference wide = new SlotReference("wide", BigIntType.INSTANCE);
Expression equality = new EqualTo(new Cast(small, BigIntType.INSTANCE), wide);
List<Expression> predicates = ImmutableList.of(
new Not(new EqualTo(small, new IntegerLiteral(10))),
new InPredicate(small, ImmutableList.of(new IntegerLiteral(10), new IntegerLiteral(20))),
new Not(new InPredicate(small,
ImmutableList.of(new IntegerLiteral(10), new IntegerLiteral(20)))));
List<Expression> expected = ImmutableList.of(
new Not(new EqualTo(wide, new BigIntLiteral(10))),
new InPredicate(wide, ImmutableList.of(new BigIntLiteral(10), new BigIntLiteral(20))),
new Not(new InPredicate(wide, ImmutableList.of(new BigIntLiteral(10), new BigIntLiteral(20)))));
for (int i = 0; i < predicates.size(); i++) {
Set<Expression> inputs = new LinkedHashSet<>(ImmutableList.of(equality, predicates.get(i)));
Assertions.assertTrue(InferPredicateByReplace.infer(inputs).contains(expected.get(i)));
}
}

@Test
public void testSameTypeBehindWideningCasts() {
SlotReference a = new SlotReference("a", IntegerType.INSTANCE);
SlotReference b = new SlotReference("b", IntegerType.INSTANCE);
Expression equality = new EqualTo(new Cast(a, BigIntType.INSTANCE), new Cast(b, BigIntType.INSTANCE));
Expression predicate = new EqualTo(new Length(new Cast(a, StringType.INSTANCE)), new IntegerLiteral(1));
Set<Expression> inputs = new LinkedHashSet<>(ImmutableList.of(equality, predicate));
Expression expected = new EqualTo(new Length(new Cast(b, StringType.INSTANCE)), new IntegerLiteral(1));
Assertions.assertTrue(InferPredicateByReplace.infer(inputs).contains(expected));
}
}
73 changes: 73 additions & 0 deletions ...-core/src/test/java/org/apache/doris/nereids/rules/rewrite/UnequalPredicateInferTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,18 @@
import org.apache.doris.nereids.types.IntegerType;
import org.apache.doris.nereids.util.PredicateInferUtils;

import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class UnequalPredicateInferTest {
Expand Down Expand Up @@ -685,4 +689,73 @@ public void testInferWithTransitiveEqualityWithCastDatev2andDate() {
EqualTo expected = new EqualTo(a, b);
Assertions.assertTrue(result.contains(expected) || result.contains(expected.commute()), "Expected to find a = b in the result.");
}

@Test
public void testInputPredicateSemantics() {
SlotReference a = new SlotReference("a", IntegerType.INSTANCE, false, ImmutableList.of("t"));
SlotReference b = new SlotReference("b", IntegerType.INSTANCE, false, ImmutableList.of("t"));
List<List<Relation>> relations = new ArrayList<>();
for (Relation first : ImmutableList.of(Relation.GT, Relation.GTE)) {
for (Relation second : ImmutableList.of(Relation.GT, Relation.GTE, Relation.EQ)) {
for (Relation third : ImmutableList.of(Relation.GT, Relation.GTE)) {
relations.add(ImmutableList.of(first, second, third));
}
}
}
relations.add(ImmutableList.of(Relation.EQ, Relation.EQ, Relation.EQ));
// Window outputs have no table qualifier. Also cover same-table and cross-table slots.
for (List<String> qualifier : ImmutableList.of(ImmutableList.<String>of(),
ImmutableList.of("t"), ImmutableList.of("other"))) {
SlotReference rn = new SlotReference("rn", IntegerType.INSTANCE, false, qualifier);
for (List<Relation> types : relations) {
List<Expression> predicates = ImmutableList.of(comparison(a, b, types.get(0)),
comparison(rn, b, types.get(1)), comparison(a, rn, types.get(2)));
for (List<Expression> permutation : Collections2.permutations(predicates)) {
Set<Expression> inputs = new LinkedHashSet<>(permutation);
Set<? extends Expression> inferred = UnequalPredicateInfer.inferUnequalPredicates(inputs);
for (int av = 0; av <= 3; av++) {
for (int bv = 0; bv <= 3; bv++) {
for (int rv = 0; rv <= 3; rv++) {
Map<Expression, Integer> values = ImmutableMap.of(a, av, b, bv, rn, rv);
boolean expected = inputs.stream().allMatch(p -> evaluateComparison(p, values));
boolean actual = inferred.stream().allMatch(p -> evaluateComparison(p, values));
Assertions.assertEquals(expected, actual,
() -> "inputs=" + inputs + ", inferred=" + inferred + ", values=" + values);
}
}
}
}
}
}
}

private static Expression comparison(Expression left, Expression right, Relation relation) {
switch (relation) {
case GT:
return new GreaterThan(left, right);
case GTE:
return new GreaterThanEqual(left, right);
case EQ:
return new EqualTo(left, right);
default:
throw new AssertionError("Unexpected relation: " + relation);
}
}

private static boolean evaluateComparison(Expression expression, Map<Expression, Integer> values) {
int left = values.get(expression.child(0));
int right = values.get(expression.child(1));
if (expression instanceof GreaterThan) {
return left > right;
} else if (expression instanceof GreaterThanEqual) {
return left >= right;
} else if (expression instanceof LessThan) {
return left < right;
} else if (expression instanceof LessThanEqual) {
return left <= right;
} else if (expression instanceof EqualTo) {
return left == right;
}
throw new AssertionError("Unexpected comparison: " + expression);
}
}
15 changes: 15 additions & 0 deletions regression-test/data/nereids_rules_p0/infer_predicate_qualify.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !strict --

-- !non_strict --
1 2 1 1
2 2 1 2

-- !mixed --
1 2 1 1

-- !reordered --

-- !matching_row --
3 4 1 3

36 changes: 36 additions & 0 deletions regression-test/data/nereids_rules_p0/infer_predicate_replace_type.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !date_length --
1 10

-- !datetime_length --
1 10

-- !date_same_type --
1 1

-- !date_comparison --
1 10

-- !signed_zero_facts --
1 true true false
2 true false true
3 true false false
4 true false false
5 true true true

-- !signed_zero_or --
1
3
5

-- !signed_zero_or_reversed --
2
3
5

-- !float_comparison --
3

-- !decimal_scale --
1 10

Loading
Loading