From d47c1c00b5189241ed2ac33d2182a8ea42503e24 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Wed, 9 Sep 2026 18:05:22 +0800 Subject: [PATCH] [fix](fe) Normalize projected count slots before null safety checks The count-on-index implementation rule checked filter safety using the aggregate's projected slot IDs. A filter below the project uses source slot IDs, so an IS NULL predicate on a counted alias could be missed. Resolve aggregate arguments through the project before applying the existing predicate checks. This keeps the checks and filters in the same slot-ID domain and prevents an unsafe storage-layer count pushdown. Issue Number: None Tests: - PhysicalStorageLayerAggregateTest - sandbox SQL reproduction with nullable indexed data and a subquery alias --- .../implementation/AggregateStrategies.java | 4 +- .../PhysicalStorageLayerAggregateTest.java | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java index ab92e8832bd0ac..906ba73052b0d9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java @@ -133,8 +133,8 @@ public List buildRules() { return false; } - Set aggSlots = funcs.stream() - .flatMap(f -> f.getInputSlots().stream()) + Set aggSlots = normalizeArguments(funcs, agg.child()).stream() + .flatMap(argument -> argument.getInputSlots().stream()) .collect(Collectors.toSet()); return aggSlots.isEmpty() || conjuncts.stream().allMatch(expr -> checkSlotInOrExpression(expr, aggSlots) && checkIsNullExpr(expr, aggSlots)); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java index 13bc470e035d28..74708104e35357 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java @@ -19,8 +19,10 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Index; import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.Type; +import org.apache.doris.catalog.info.IndexType; import org.apache.doris.datasource.CatalogIf; import org.apache.doris.datasource.plugin.PluginDrivenExternalTable; import org.apache.doris.nereids.CascadesContext; @@ -29,6 +31,7 @@ import org.apache.doris.nereids.rules.RuleType; import org.apache.doris.nereids.rules.implementation.AggregateStrategies; import org.apache.doris.nereids.trees.expressions.Alias; +import org.apache.doris.nereids.trees.expressions.IsNull; 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; @@ -37,6 +40,7 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan.SelectedPartitions; +import org.apache.doris.nereids.trees.plans.logical.LogicalFilter; import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan; import org.apache.doris.nereids.trees.plans.logical.LogicalProject; import org.apache.doris.nereids.trees.plans.physical.PhysicalStorageLayerAggregate.PushDownAggOp; @@ -46,6 +50,7 @@ import org.apache.doris.nereids.util.PlanConstructor; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -259,6 +264,31 @@ public void testWithProject() { ); } + @Test + public void testCountOnIndexRejectsIsNullOnProjectedCountSlot() { + LogicalOlapScan olapScan = PlanConstructor.newLogicalOlapScan(2, "count_alias", 0); + Index invertedIndex = new Index(1L, "idx_name", ImmutableList.of("name"), + IndexType.INVERTED, null, ""); + olapScan.getTable().getIndexIdToMeta().values().forEach( + meta -> meta.setIndexes(ImmutableList.of(invertedIndex))); + + LogicalFilter filter = new LogicalFilter<>( + ImmutableSet.of(new IsNull(olapScan.getOutput().get(1))), olapScan); + LogicalProject> project = new LogicalProject<>( + ImmutableList.of(new Alias(olapScan.getOutput().get(1), "x")), filter); + LogicalAggregate>> aggregate = new LogicalAggregate<>( + Collections.emptyList(), + ImmutableList.of(new Alias(new Count(project.getOutput().get(0)), "count_x"), + new Alias(new Count(), "count_star")), + true, Optional.empty(), project); + CascadesContext context = MemoTestUtils.createCascadesContext(aggregate); + context.getConnectContext().getSessionVariable().setEnablePushDownCountOnIndex(true); + + PlanChecker.from(context) + .applyImplementation(countOnIndex()) + .matches(logicalAggregate(logicalProject(logicalFilter(logicalOlapScan())))); + } + @Test void testProjectionCheck() { LogicalOlapScan olapScan = PlanConstructor.newLogicalOlapScan(1, "tbl", 0); @@ -309,4 +339,12 @@ private Rule storageLayerAggregateWithProject() { .findFirst() .get(); } + + private Rule countOnIndex() { + return new AggregateStrategies().buildRules() + .stream() + .filter(rule -> rule.getRuleType() == RuleType.COUNT_ON_INDEX) + .findFirst() + .get(); + } }