From 63f45c6eb6fc4b12208bd3cbc7bfb791dd4e60bd Mon Sep 17 00:00:00 2001 From: yujun Date: Thu, 8 Jan 2026 15:41:25 +0800 Subject: [PATCH 1/2] fix compound value merge --- .../expression/rules/RangeInference.java | 22 +++++++++++++------ .../rules/expression/SimplifyRangeTest.java | 12 +++++++++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java index 25541f7fe0b813..8b4621fb81ec29 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java @@ -198,6 +198,7 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List valuePerRefs = Lists.newArrayList(); Map groupByReference = Maps.newLinkedHashMap(); for (Expression predicate : predicates) { // given an expression A, no matter A is nullable or not, @@ -213,11 +214,16 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List new ValueDescCollector()).add(valueDesc); + + // valueDesc is like 'a > 1 and b > 1', don't merge them + if (valueDesc instanceof CompoundValue && !((CompoundValue) valueDesc).isSameReference) { + valuePerRefs.add(valueDesc); + } else { + Expression reference = valueDesc.reference; + groupByReference.computeIfAbsent(reference, key -> new ValueDescCollector()).add(valueDesc); + } } - List valuePerRefs = Lists.newArrayList(); for (Entry referenceValues : groupByReference.entrySet()) { Expression reference = referenceValues.getKey(); ValueDescCollector collector = referenceValues.getValue(); @@ -465,10 +471,7 @@ private Optional mergeCompoundValues(ExpressionRewriteContext context // if A's range is bigger than B, then A or (B and C) = A // if A or B is true/all, then A or (B and C) = A or C for (CompoundValue compoundValue : collector.compoundValues) { - if (isAnd != compoundValue.isAnd - && compoundValue.reference.equals(reference) - // no process the compose value which reference different - && compoundValue.sourceValues.get(0).reference.equals(reference)) { + if (isAnd != compoundValue.isAnd && compoundValue.reference.equals(reference)) { ImmutableList.Builder newSourceValuesBuilder = ImmutableList.builderWithExpectedSize(compoundValue.sourceValues.size()); boolean skipWholeCompoundValue = false; @@ -1223,6 +1226,7 @@ public static class CompoundValue extends ValueDesc { private final Set> subClasses; private final boolean hasNullable; private final boolean hasNoneNullable; + private final boolean isSameReference; /** constructor */ public CompoundValue(ExpressionRewriteContext context, Expression reference, @@ -1234,20 +1238,24 @@ public CompoundValue(ExpressionRewriteContext context, Expression reference, this.subClasses.add(getClass()); boolean hasNullable = false; boolean hasNonNullable = false; + boolean isSameReference = true; for (ValueDesc sourceValue : sourceValues) { if (sourceValue instanceof CompoundValue) { CompoundValue compoundSource = (CompoundValue) sourceValue; this.subClasses.addAll(compoundSource.subClasses); hasNullable = hasNullable || compoundSource.hasNullable; hasNonNullable = hasNonNullable || compoundSource.hasNoneNullable; + isSameReference = isSameReference && compoundSource.isSameReference; } else { this.subClasses.add(sourceValue.getClass()); hasNullable = hasNullable || sourceValue.nullable(); hasNonNullable = hasNonNullable || !sourceValue.nullable(); } + isSameReference = isSameReference && sourceValue.getReference().equals(reference); } this.hasNullable = hasNullable; this.hasNoneNullable = hasNonNullable; + this.isSameReference = isSameReference; } public List getSourceValues() { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java index 66d39739f03bf8..0d65322c8bbbd6 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java @@ -307,7 +307,7 @@ public void testSimplifyNumeric() { assertRewrite("TA > 5 + 1 and TA > 10", "cast(TA as smallint) > 6 and TA > 10"); assertRewrite("(TA > 1 and TA > 10) or TA > 20", "TA > 10"); assertRewrite("(TA > 1 or TA > 10) and TA > 20", "TA > 20"); - assertRewrite("(TA < 1 and TA > 10) or TA = 20 and TB > 10", "(TA is null and null) or TA = 20 and TB > 10"); + assertRewrite("(TA < 1 and TA > 10) or TA = 20 and TB > 10", "TA = 20 and TB > 10 or (TA is null and null) "); assertRewrite("(TA + TB > 1 or TA + TB > 10) and TA + TB > 20", "TA + TB > 20"); assertRewrite("TA > 10 or TA > 10", "TA > 10"); assertRewrite("(TA > 10 or TA > 20) and (TB > 10 and TB < 20)", "TA > 10 and (TB > 10 and TB < 20) "); @@ -673,6 +673,16 @@ public void testSimplifyDateTime() { "(CA is null and null) OR CB < timestamp '2024-01-05 00:50:00'"); } + @Test + public void testMixTypes() { + executor = new ExpressionRuleExecutor(ImmutableList.of( + bottomUp(SimplifyRange.INSTANCE) + )); + assertRewrite("(TA > 1 and FALSE or FALSE and SA > 'aaaa') and TB is null", "FALSE"); + assertRewrite("(TA > 1 and FALSE or FALSE and SA > 'aaaa') and (TA > 1 and FALSE or FALSE and SA > 'aaaa') and TB is null", + "FALSE"); + } + private ValueDesc getValueDesc(String expression) { Expression parseExpression = replaceUnboundSlot(PARSER.parseExpression(expression), commonMem); parseExpression = typeCoercion(parseExpression); From 05feb05024464859a6c32d12dba5cdc634377748 Mon Sep 17 00:00:00 2001 From: yujun Date: Thu, 8 Jan 2026 17:33:13 +0800 Subject: [PATCH 2/2] update --- .../expression/rules/RangeInference.java | 26 ++++++++++++------- .../rules/expression/SimplifyRangeTest.java | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java index 8b4621fb81ec29..d63f2bad43d742 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java @@ -17,6 +17,7 @@ package org.apache.doris.nereids.rules.expression.rules; +import org.apache.doris.common.Pair; import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext; import org.apache.doris.nereids.trees.expressions.And; import org.apache.doris.nereids.trees.expressions.EqualTo; @@ -198,8 +199,8 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List valuePerRefs = Lists.newArrayList(); - Map groupByReference = Maps.newLinkedHashMap(); + Map, ValueDescCollector> groupByReference = Maps.newLinkedHashMap(); + int nextUniqueNum = 1; for (Expression predicate : predicates) { // given an expression A, no matter A is nullable or not, // 'A is null and null' can represent as EmptyValue(A), @@ -215,17 +216,24 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List 1 and b > 1', don't merge them + int uniqueNum = 0; + + // for compound value with diff source value reference like 'a > 1 and b > 1', + // don't merge it with other values, so give them a unique num > 0. + // for other value desc, their unique num is always 0. if (valueDesc instanceof CompoundValue && !((CompoundValue) valueDesc).isSameReference) { - valuePerRefs.add(valueDesc); - } else { - Expression reference = valueDesc.reference; - groupByReference.computeIfAbsent(reference, key -> new ValueDescCollector()).add(valueDesc); + nextUniqueNum++; + uniqueNum = nextUniqueNum; } + + Expression reference = valueDesc.reference; + groupByReference.computeIfAbsent(Pair.of(reference, uniqueNum), + key -> new ValueDescCollector()).add(valueDesc); } - for (Entry referenceValues : groupByReference.entrySet()) { - Expression reference = referenceValues.getKey(); + List valuePerRefs = Lists.newArrayList(); + for (Entry, ValueDescCollector> referenceValues : groupByReference.entrySet()) { + Expression reference = referenceValues.getKey().first; ValueDescCollector collector = referenceValues.getValue(); ValueDesc mergedValue; if (isAnd) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java index 0d65322c8bbbd6..3d9341d2b0cbb7 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyRangeTest.java @@ -307,7 +307,7 @@ public void testSimplifyNumeric() { assertRewrite("TA > 5 + 1 and TA > 10", "cast(TA as smallint) > 6 and TA > 10"); assertRewrite("(TA > 1 and TA > 10) or TA > 20", "TA > 10"); assertRewrite("(TA > 1 or TA > 10) and TA > 20", "TA > 20"); - assertRewrite("(TA < 1 and TA > 10) or TA = 20 and TB > 10", "TA = 20 and TB > 10 or (TA is null and null) "); + assertRewrite("(TA < 1 and TA > 10) or TA = 20 and TB > 10", "(TA is null and null) or TA = 20 and TB > 10"); assertRewrite("(TA + TB > 1 or TA + TB > 10) and TA + TB > 20", "TA + TB > 20"); assertRewrite("TA > 10 or TA > 10", "TA > 10"); assertRewrite("(TA > 10 or TA > 20) and (TB > 10 and TB < 20)", "TA > 10 and (TB > 10 and TB < 20) ");