This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 7cd9e8b1360 branch-4.2: [fix](aggregate) Guard aggregate not-null
inference by null semantics #67668 (#70071)
7cd9e8b1360 is described below
commit 7cd9e8b1360a502adf7427eb282343aaba22ee1e
Author: morrySnow <[email protected]>
AuthorDate: Thu Nov 5 09:40:30 2026 +0800
branch-4.2: [fix](aggregate) Guard aggregate not-null inference by null
semantics #67668 (#70071)
picked from master #67668
---
.../nereids/rules/rewrite/InferAggNotNull.java | 23 ++------
.../apache/doris/nereids/util/ExpressionUtils.java | 60 +++++++++++++++------
.../nereids/rules/rewrite/InferAggNotNullTest.java | 24 ++++++++-
.../eliminate_aggregate_casewhen.out | 3 +-
.../infer_agg_not_null/infer_agg_not_null.out | 12 +++++
.../infer_agg_not_null/infer_agg_not_null.groovy | 63 ++++++++++++++++++++++
6 files changed, 150 insertions(+), 35 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
index e27a7b8cfad..f5fc26cb436 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
@@ -23,11 +23,7 @@ import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.Not;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Avg;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Max;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Min;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
+import
org.apache.doris.nereids.trees.expressions.functions.agg.NullIgnoringAggregateFunction;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Filter;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
@@ -40,9 +36,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
-/**
- * InferNotNull from Agg count(distinct);
- */
+/** Infer not-null predicates from null-ignoring global aggregate arguments. */
public class InferAggNotNull extends OneRewriteRuleFactory {
@Override
public Rule build() {
@@ -103,20 +97,13 @@ public class InferAggNotNull extends OneRewriteRuleFactory
{
private Set<Expression> inferFunctionNotNulls(
AggregateFunction aggregateFunction, CascadesContext
cascadesContext) {
- return
ExpressionUtils.inferNotNull(ImmutableSet.copyOf(aggregateFunction.children()),
cascadesContext);
+ return ExpressionUtils.inferNotNullForNullIgnoringAggregate(
+ ImmutableSet.copyOf(aggregateFunction.children()),
cascadesContext);
}
private boolean canInferFunctionNotNull(AggregateFunction
aggregateFunction) {
- return isSupportedAggregateFunction(aggregateFunction)
+ return aggregateFunction instanceof NullIgnoringAggregateFunction
&& !aggregateFunction.children().isEmpty();
}
- private boolean isSupportedAggregateFunction(AggregateFunction
aggregateFunction) {
- return aggregateFunction instanceof Count
- || aggregateFunction instanceof Avg
- || aggregateFunction instanceof Sum
- || aggregateFunction instanceof Max
- || aggregateFunction instanceof Min;
- }
-
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
index 11167e70a95..07c5e16495c 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
@@ -1002,15 +1002,7 @@ public class ExpressionUtils {
* infer notNulls slot from predicate
*/
public static Set<Slot> inferNotNullSlots(Set<Expression> predicates,
CascadesContext cascadesContext) {
- Set<Slot> targetSlots = new HashSet<>();
- for (Expression predicate : predicates) {
- for (Slot slot : predicate.getInputSlots()) {
- if (!(slot instanceof MarkJoinSlotReference)) {
- targetSlots.add(slot);
- }
- }
- }
- return inferNotNullSlots(predicates, targetSlots, cascadesContext);
+ return inferNotNullSlots(predicates,
collectNotNullInferenceTargetSlots(predicates), cascadesContext);
}
/**
@@ -1018,6 +1010,11 @@ public class ExpressionUtils {
*/
public static Set<Slot> inferNotNullSlots(Set<Expression> predicates,
Set<Slot> targetSlots,
CascadesContext cascadesContext) {
+ return inferNotNullSlots(predicates, targetSlots, cascadesContext,
ExpressionUtils::isFalseOrNull);
+ }
+
+ private static Set<Slot> inferNotNullSlots(Set<Expression> predicates,
Set<Slot> targetSlots,
+ CascadesContext cascadesContext, Predicate<Expression>
nullInputResultPredicate) {
ImmutableSet.Builder<Slot> notNullSlots =
ImmutableSet.builderWithExpectedSize(targetSlots.size());
Set<Slot> inputSlots = new HashSet<>();
for (Expression predicate : predicates) {
@@ -1036,7 +1033,7 @@ public class ExpressionUtils {
}
inputSlots = mergedInputSlots.get();
for (Slot slot : candidateSlots) {
- if (isNullRejecting(predicate, slot, cascadesContext)) {
+ if (matchesWhenSlotIsNull(predicate, slot, cascadesContext,
nullInputResultPredicate)) {
notNullSlots.add(slot);
}
}
@@ -1044,14 +1041,27 @@ public class ExpressionUtils {
return notNullSlots.build();
}
- private static boolean isNullRejecting(Expression predicate, Slot slot,
CascadesContext cascadesContext) {
+ private static Set<Slot>
collectNotNullInferenceTargetSlots(Set<Expression> expressions) {
+ Set<Slot> targetSlots = new HashSet<>();
+ for (Expression expression : expressions) {
+ for (Slot slot : expression.getInputSlots()) {
+ if (!(slot instanceof MarkJoinSlotReference)) {
+ targetSlots.add(slot);
+ }
+ }
+ }
+ return targetSlots;
+ }
+
+ private static boolean matchesWhenSlotIsNull(Expression expression, Slot
slot, CascadesContext cascadesContext,
+ Predicate<Expression> nullInputResultPredicate) {
Map<Expression, Expression> replaceMap = new HashMap<>();
Literal nullLiteral = new NullLiteral(slot.getDataType());
replaceMap.put(slot, nullLiteral);
Expression evalExpr = FoldConstantRule.evaluate(
- ExpressionUtils.replace(predicate, replaceMap),
+ ExpressionUtils.replace(expression, replaceMap),
new ExpressionRewriteContext(cascadesContext));
- return evalExpr.isNullLiteral() ||
BooleanLiteral.FALSE.equals(evalExpr);
+ return nullInputResultPredicate.apply(evalExpr);
}
private static Optional<Set<Slot>> mergeInputSlotsWithinLimit(Set<Slot>
inputSlots, Set<Slot> predicateInputSlots) {
@@ -1070,8 +1080,28 @@ public class ExpressionUtils {
* infer notNulls slot from predicate
*/
public static Set<Expression> inferNotNull(Set<Expression> predicates,
CascadesContext cascadesContext) {
- ImmutableSet.Builder<Expression> newPredicates =
ImmutableSet.builderWithExpectedSize(predicates.size());
- for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) {
+ return buildNotNullPredicates(inferNotNullSlots(predicates,
cascadesContext));
+ }
+
+ /**
+ * Infer not-null predicates for an aggregate that ignores rows with SQL
NULL arguments.
+ *
+ * <p>The caller must first establish the aggregate's null-input contract.
Even for a
+ * null-ignoring aggregate, a row can only be discarded when its argument
evaluates to SQL
+ * NULL. FALSE is null-rejecting as a filter predicate, but it is a valid
aggregate argument
+ * and must be kept.
+ */
+ public static Set<Expression> inferNotNullForNullIgnoringAggregate(
+ Set<Expression> arguments, CascadesContext cascadesContext) {
+ Set<Slot> targetSlots = collectNotNullInferenceTargetSlots(arguments);
+ Set<Slot> notNullSlots = inferNotNullSlots(
+ arguments, targetSlots, cascadesContext,
Expression::isNullLiteral);
+ return buildNotNullPredicates(notNullSlots);
+ }
+
+ private static Set<Expression> buildNotNullPredicates(Set<Slot>
notNullSlots) {
+ ImmutableSet.Builder<Expression> newPredicates =
ImmutableSet.builderWithExpectedSize(notNullSlots.size());
+ for (Slot slot : notNullSlots) {
newPredicates.add(new Not(new IsNull(slot), false));
}
return newPredicates.build();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
index c127f7e40ad..f0e709dc1b7 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
@@ -37,6 +37,8 @@ import
org.apache.doris.nereids.trees.expressions.functions.agg.CountByEnum;
import org.apache.doris.nereids.trees.expressions.functions.agg.GroupConcat;
import org.apache.doris.nereids.trees.expressions.functions.agg.MapAgg;
import org.apache.doris.nereids.trees.expressions.functions.agg.MapAggV2;
+import org.apache.doris.nereids.trees.expressions.functions.agg.Max;
+import org.apache.doris.nereids.trees.expressions.functions.agg.Min;
import
org.apache.doris.nereids.trees.expressions.functions.agg.NullIgnoringAggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum0;
@@ -79,13 +81,33 @@ class InferAggNotNullTest implements
MemoPatternMatchSupported {
);
}
+ @Test
+ void testNotInferWhenAggregateArgumentReturnsFalseForNullInput() {
+ Expression isNotNull = new Not(new IsNull(scan1.getOutput().get(1)));
+ LogicalPlan plan = new LogicalPlanBuilder(scan1)
+ .aggGroupUsingIndex(ImmutableList.of(),
+ ImmutableList.of(new Alias(new Count(false,
isNotNull), "cnt")))
+ .build();
+
+ PlanChecker.from(MemoTestUtils.createConnectContext(), plan)
+ .applyTopDown(new InferAggNotNull())
+ .matches(
+ logicalAggregate(
+ logicalOlapScan()
+ )
+ );
+ }
+
@Test
void testInferMultipleAggregateSameInput() {
LogicalPlan plan = new LogicalPlanBuilder(scan1)
.aggGroupUsingIndex(ImmutableList.of(),
ImmutableList.of(
+ new Alias(new Count(false,
scan1.getOutput().get(1)), "count_k"),
new Alias(new Avg(scan1.getOutput().get(1)),
"avg_k"),
- new Alias(new Sum(scan1.getOutput().get(1)),
"sum_k")))
+ new Alias(new Sum(scan1.getOutput().get(1)),
"sum_k"),
+ new Alias(new Max(scan1.getOutput().get(1)),
"max_k"),
+ new Alias(new Min(scan1.getOutput().get(1)),
"min_k")))
.build();
PlanChecker.from(MemoTestUtils.createConnectContext(), plan)
diff --git
a/regression-test/data/nereids_rules_p0/eliminate_aggregate_casewhen/eliminate_aggregate_casewhen.out
b/regression-test/data/nereids_rules_p0/eliminate_aggregate_casewhen/eliminate_aggregate_casewhen.out
index 4de89d48061..df19fb31688 100644
---
a/regression-test/data/nereids_rules_p0/eliminate_aggregate_casewhen/eliminate_aggregate_casewhen.out
+++
b/regression-test/data/nereids_rules_p0/eliminate_aggregate_casewhen/eliminate_aggregate_casewhen.out
@@ -200,7 +200,8 @@ PhysicalResultSink
--------hashAgg[GLOBAL]
----------hashAgg[LOCAL]
------------PhysicalProject
---------------PhysicalOlapScan[t4]
+--------------filter((t4.c4 = 102))
+----------------PhysicalOlapScan[t4]
-- !bool_or_eliminate --
PhysicalResultSink
diff --git
a/regression-test/data/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.out
b/regression-test/data/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.out
index feb9fa5c982..273ae0d5cac 100644
---
a/regression-test/data/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.out
+++
b/regression-test/data/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.out
@@ -8,3 +8,15 @@
-- !null_sensitive_aggregates_keep_rows --
3 3
+-- !count_boolean_argument --
+3
+
+-- !count_nullable_slot --
+1
+
+-- !count_coalesce_argument --
+3
+
+-- !mixed_count_array_agg --
+1 3
+
diff --git
a/regression-test/suites/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.groovy
b/regression-test/suites/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.groovy
index 056287766cd..2537a261aea 100644
---
a/regression-test/suites/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.groovy
+++
b/regression-test/suites/nereids_rules_p0/infer_agg_not_null/infer_agg_not_null.groovy
@@ -73,4 +73,67 @@ suite("infer_agg_not_null") {
"""
contains "LEFT_OUTER_JOIN"
}
+ order_qt_count_boolean_argument """
+ SELECT COUNT(r.v IS NOT NULL)
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+
+ order_qt_count_nullable_slot """
+ SELECT COUNT(r.v)
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+
+ order_qt_count_coalesce_argument """
+ SELECT COUNT(COALESCE(r.v, 0))
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+
+ order_qt_mixed_count_array_agg """
+ SELECT COUNT(r.v), ARRAY_SIZE(ARRAY_AGG(r.v))
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+
+ explain {
+ sql """
+ SHAPE PLAN
+ SELECT COUNT(r.v IS NOT NULL)
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+ contains "LEFT_OUTER_JOIN"
+ }
+
+ explain {
+ sql """
+ SHAPE PLAN
+ SELECT COUNT(r.v)
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+ contains "INNER_JOIN"
+ }
+
+ explain {
+ sql """
+ SHAPE PLAN
+ SELECT COUNT(COALESCE(r.v, 0))
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+ contains "LEFT_OUTER_JOIN"
+ }
+
+ explain {
+ sql """
+ SHAPE PLAN
+ SELECT COUNT(r.v), ARRAY_SIZE(ARRAY_AGG(r.v))
+ FROM infer_agg_not_null_left l
+ LEFT JOIN infer_agg_not_null_right r ON l.k = r.k
+ """
+ contains "LEFT_OUTER_JOIN"
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
