This is an automated email from the ASF dual-hosted git repository.
morrySnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 5a84d88a9c7 [fix](aggregate) Normalize projected count slots before
null safety checks (#67732)
5a84d88a9c7 is described below
commit 5a84d88a9c7b9bb359019494fa58db836733e4c9
Author: morrySnow <[email protected]>
AuthorDate: Thu Sep 10 12:14:07 2026 +0800
[fix](aggregate) Normalize projected count slots before null safety checks
(#67732)
## Problem
Counting a projected alias over a nullable indexed column can return an
incorrect non-zero result when the filter retains only null rows. Mixing
the alias count with `COUNT(*)` or another count exposes the problem:
```sql
SELECT COUNT(x), COUNT(*)
FROM (SELECT k AS x FROM t WHERE k IS NULL) q;
```
For two matching null rows, the correct result is `(0, 2)`, but the
storage-layer index-count path can return `(2, 2)`.
## Root cause
The FE implementation rule validates `IS NULL` and OR predicates before
pushing count aggregation to the storage layer. In the Project variant,
this validation used the aggregate-side alias slot, while the filter
below the Project refers to the source slot. Their expression IDs
differ, so the null-safety guard did not recognize that the filter and
`COUNT` referenced the same nullable value.
The rule normalized the aggregate argument to the source slot only
later, after the safety decision had already been made.
## Reproduction
```sql
CREATE TABLE t (
id INT NOT NULL,
k INT NULL,
INDEX idx_k (k) USING INVERTED
)
DUPLICATE KEY(id)
DISTRIBUTED BY HASH(id) BUCKETS 1
PROPERTIES ("replication_num" = "1");
INSERT INTO t VALUES (1, NULL), (2, NULL), (3, 1);
SELECT COUNT(x), COUNT(*)
FROM (SELECT k AS x FROM t WHERE k IS NULL) q;
SELECT COUNT(x), COUNT(id)
FROM (SELECT k AS x, id FROM t WHERE k IS NULL) q;
```
Before this change, both queries return `(2, 2)` and the plan contains
`pushAggOp=COUNT_ON_INDEX`. Both queries should return `(0, 2)`.
## Fix
Normalize aggregate arguments through the Project before collecting the
slots used by the predicate safety checks. The count slots and filter
slots are now compared in the same source expression-ID domain. If `IS
NULL` targets a counted source slot, the FE rejects the index-count
pushdown and preserves the column's null values.
This change is limited to the FE planner.
## Tests
- Added a FE plan test for `COUNT(projected_alias) + COUNT(*)` above an
`IS NULL` filter, verifying that the count-on-index implementation rule
is rejected.
- Ran `PhysicalStorageLayerAggregateTest`: 7 tests passed.
- Deployed the FE to a local sandbox and reran both SQL reproductions.
They return `(0, 2)`, and the scan plan reports `pushAggOp=NONE`.
---
.../rules/implementation/AggregateStrategies.java | 4 +--
.../rewrite/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 ab92e8832bd..906ba73052b 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 class AggregateStrategies implements
ImplementationRuleFactory {
return false;
}
- Set<Slot> aggSlots = funcs.stream()
- .flatMap(f -> f.getInputSlots().stream())
+ Set<Slot> 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 13bc470e035..74708104e35 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 @@ package org.apache.doris.nereids.rules.rewrite;
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.RulePromise;
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.RelationId;
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.PlanChecker;
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 class PhysicalStorageLayerAggregateTest implements
MemoPatternMatchSuppor
);
}
+ @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<LogicalOlapScan> filter = new LogicalFilter<>(
+ ImmutableSet.of(new IsNull(olapScan.getOutput().get(1))),
olapScan);
+ LogicalProject<LogicalFilter<LogicalOlapScan>> project = new
LogicalProject<>(
+ ImmutableList.of(new Alias(olapScan.getOutput().get(1), "x")),
filter);
+ LogicalAggregate<LogicalProject<LogicalFilter<LogicalOlapScan>>>
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 @@ public class PhysicalStorageLayerAggregateTest implements
MemoPatternMatchSuppor
.findFirst()
.get();
}
+
+ private Rule countOnIndex() {
+ return new AggregateStrategies().buildRules()
+ .stream()
+ .filter(rule -> rule.getRuleType() == RuleType.COUNT_ON_INDEX)
+ .findFirst()
+ .get();
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
