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 341a312f5f1 [fix](fd) Guard uniform aggregate inference by
participation (#67881)
341a312f5f1 is described below
commit 341a312f5f1c9105d10cd174a3c0bab50cea7ffe
Author: morrySnow <[email protected]>
AuthorDate: Mon Sep 14 18:39:17 2026 +0800
[fix](fd) Guard uniform aggregate inference by participation (#67881)
## Problem
When an aggregate groups by a unique, non-null key, each group contains
one row. The planner used that fact to mark every `COUNT` and `NDV`
output as uniform. That is not true for nullable arguments: `COUNT(v)`
and `NDV(v)` return `0` for a null value and `1` for a non-null value.
An outer aggregation can consequently remove such an output from its
group keys and merge rows that must remain separate.
## Root cause
The logical and physical aggregate trait derivations classified an
output as uniform solely from the aggregate function class. They did not
distinguish `COUNT(*)` from argument-based aggregates or check whether
the complete argument expressions always participate in the aggregate.
## Reproduction
Create a unique-key table containing two rows whose nullable value
differs:
```sql
create table uniform_agg_witness (
pk int not null,
b int not null,
v int null
) unique key(pk)
distributed by hash(pk) buckets 1
properties("replication_num"="1");
insert into uniform_agg_witness values (1, 7, null), (2, 7, 9);
select b, c, count(*) as n, sum(h) as sh
from (
select pk, b, count(v) as c, ndv(v) as h
from uniform_agg_witness
group by pk, b
) s
group by b, c
order by b, c;
```
The invalid uniform trait removed `c` from the outer group keys and
produced one merged row. The correct result has separate `(7, 0)` and
`(7, 1)` groups.
## Fix
- Share one uniform-aggregate proof between logical and physical
aggregate plans.
- Keep `COUNT(*)` uniform for a single-row group.
- Treat argument-based `COUNT` and `NDV` as uniform only when every
complete argument expression is definitely non-null.
- Default all other cases to non-uniform. This conservatively rejects
nullable arguments, nullable conditional expressions, narrowing and try
casts whose result may be null, multi-argument counts with any nullable
argument, and null-extended outer-join outputs.
- Preserve the safe optimization for non-null arguments.
## Tests
- `./run-fe-ut.sh --run org.apache.doris.nereids.properties.UniformTest`
(12 tests passed)
- `./build.sh --fe`
- `./run-regression-test.sh --run -f
regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy
... -forceGenOut`
- Re-ran the same regression suite normally against the generated
expected output.
The regression asserts both results and plan group keys: nullable
`COUNT`/`NDV` remain in the outer grouping, while non-null `COUNT` still
permits safe group-key elimination.
---
.../trees/plans/logical/LogicalAggregate.java | 4 +-
.../plans/physical/PhysicalHashAggregate.java | 4 +-
.../apache/doris/nereids/util/ExpressionUtils.java | 20 +++++
.../doris/nereids/properties/UniformTest.java | 34 ++++++++
.../eliminate_group_by_key_by_uniform.out | 15 ++++
.../eliminate_group_by_key_by_uniform.groovy | 93 +++++++++++++++++++++-
6 files changed, 163 insertions(+), 7 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java
index 6eb00636a50..786daddca1f 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java
@@ -29,8 +29,6 @@ import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.functions.ExpressionTrait;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.AggregatePhase;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Ndv;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.plans.AbstractPlan;
import org.apache.doris.nereids.trees.plans.Plan;
@@ -462,7 +460,7 @@ public class LogicalAggregate<CHILD_TYPE extends Plan>
return false;
}
Expression agg = namedExpression.child(0);
- return agg instanceof Count || agg instanceof Ndv;
+ return ExpressionUtils.isUniformAgg(agg);
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
index e901998f44e..a02de15ec37 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
@@ -29,8 +29,6 @@ import
org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateParam;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
-import org.apache.doris.nereids.trees.expressions.functions.agg.Ndv;
import
org.apache.doris.nereids.trees.expressions.functions.agg.NullableAggregateFunction;
import org.apache.doris.nereids.trees.plans.AbstractPlan;
import org.apache.doris.nereids.trees.plans.AggMode;
@@ -409,7 +407,7 @@ public class PhysicalHashAggregate<CHILD_TYPE extends Plan>
extends PhysicalUnar
return false;
}
Expression agg = namedExpression.child(0);
- return agg instanceof Count || agg instanceof Ndv;
+ return ExpressionUtils.isUniformAgg(agg);
}
@Override
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 00f5caa8364..250263c3b99 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
@@ -56,8 +56,10 @@ import
org.apache.doris.nereids.trees.expressions.WindowExpression;
import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
import
org.apache.doris.nereids.trees.expressions.functions.NoneMovableFunction;
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.Ndv;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
import org.apache.doris.nereids.trees.expressions.functions.generator.Explode;
import
org.apache.doris.nereids.trees.expressions.functions.generator.ExplodeBitmap;
@@ -1210,6 +1212,24 @@ public class ExpressionUtils {
return agg instanceof Sum || agg instanceof Avg || agg instanceof Max
|| agg instanceof Min;
}
+ /**
+ * Whether a single-row group always produces the same aggregate result.
+ *
+ * <p>COUNT(*) always consumes its only row. Argument-based COUNT and NDV
consume the row only
+ * when every argument is non-null, so nullable arguments may produce
either zero or one across
+ * otherwise single-row groups. Keep the proof conservative and inspect
the complete argument
+ * expressions rather than only their input slots.</p>
+ */
+ public static boolean isUniformAgg(Expression agg) {
+ if (agg instanceof Count && ((Count) agg).isCountStar()) {
+ return true;
+ }
+ if (!(agg instanceof Count || agg instanceof Ndv)) {
+ return false;
+ }
+ return agg.getArguments().stream().allMatch(Expression::notNullable);
+ }
+
public static <E> Set<E> mutableCollect(List<? extends Expression>
expressions,
Predicate<TreeNode<Expression>> predicate) {
Set<E> set = new HashSet<>();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/UniformTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/UniformTest.java
index 77395258281..4349b2e7a2e 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/UniformTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/UniformTest.java
@@ -45,6 +45,13 @@ class UniformTest extends TestWithFeService {
+ "UNIQUE KEY(id)\n"
+ "distributed by hash(id) buckets 10\n"
+ "properties('replication_num' = '1');");
+ createTable("create table test.uniform_agg_witness (\n"
+ + "pk int not null,\n"
+ + "b int not null,\n"
+ + "v int null)\n"
+ + "UNIQUE KEY(pk)\n"
+ + "distributed by hash(pk) buckets 10\n"
+ + "properties('replication_num' = '1');");
connectContext.setDatabase("test");
connectContext.getSessionVariable().setDisableNereidsRules("PRUNE_EMPTY_PARTITION");
}
@@ -79,6 +86,33 @@ class UniformTest extends TestWithFeService {
}
+ @Test
+ void testSingleRowAggregateUniformityRequiresStableParticipation() {
+ assertAggregateUniform("select count(*) from uniform_agg_witness group
by pk", true);
+ assertAggregateUniform("select count(b) from uniform_agg_witness group
by pk", true);
+ assertAggregateUniform("select ndv(b) from uniform_agg_witness group
by pk", true);
+ assertAggregateUniform("select count(distinct pk, b) from
uniform_agg_witness group by pk", true);
+ assertAggregateUniform("select count(if(v is null, 1, 0)) from
uniform_agg_witness group by pk", true);
+
+ assertAggregateUniform("select count(v) from uniform_agg_witness group
by pk", false);
+ assertAggregateUniform("select ndv(v) from uniform_agg_witness group
by pk", false);
+ assertAggregateUniform("select count(distinct b, v) from
uniform_agg_witness group by pk", false);
+ assertAggregateUniform(
+ "select count(if(v is null, 1, null)) from uniform_agg_witness
group by pk", false);
+ assertAggregateUniform("select count(cast(pk as tinyint)) from
uniform_agg_witness group by pk", false);
+ assertAggregateUniform("select count(try_cast(pk as tinyint)) from
uniform_agg_witness group by pk", false);
+ assertAggregateUniform("select count(u.name) from uniform_agg_witness
w "
+ + "left join uni u on w.pk = u.id group by w.pk", false);
+ }
+
+ private void assertAggregateUniform(String sql, boolean expected) {
+ Plan plan = PlanChecker.from(connectContext)
+ .analyze(sql)
+ .getPlan();
+ Assertions.assertEquals(expected,
plan.getLogicalProperties().getTrait()
+ .isUniform(plan.getOutput().get(0)), sql);
+ }
+
@Test
void testTopNLimit() {
Plan plan = PlanChecker.from(connectContext)
diff --git
a/regression-test/data/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.out
b/regression-test/data/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.out
index 32e744e1630..61fe14fb171 100644
---
a/regression-test/data/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.out
+++
b/regression-test/data/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.out
@@ -8,6 +8,21 @@
-- !tranform_to_scalar_agg_not_null_column --
+-- !nullable_count_not_uniform --
+7 0 1 0
+7 1 1 1
+
+-- !nullable_ndv_not_uniform --
+7 0 1 0
+7 1 1 1
+
+-- !multi_argument_count_not_uniform --
+7 0 1
+7 1 1
+
+-- !non_nullable_count_uniform --
+7 1 2
+
-- !project_const --
\N 1
\N 1
diff --git
a/regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy
b/regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy
index 4b5b63ead8f..3d7bef31f1c 100644
---
a/regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy
+++
b/regression-test/suites/nereids_rules_p0/eliminate_gby_key/eliminate_group_by_key_by_uniform.groovy
@@ -42,6 +42,97 @@ suite("eliminate_group_by_key_by_uniform") {
qt_empty_tranform_multi_column "select a, min(a), sum(a), count(a) from
eli_gbk_by_uniform_t where a = 1 group by a, b,'abc' order by 1,2,3,4"
qt_tranform_to_scalar_agg_not_null_column "select b, min(a), sum(a),
count(a) from eli_gbk_by_uniform_t where b = 1 group by a, b order by 1,2,3,4"
+ sql "drop table if exists uniform_agg_witness"
+ sql """
+ create table uniform_agg_witness (
+ pk int not null,
+ b int not null,
+ v int null
+ ) unique key(pk)
+ distributed by hash(pk) buckets 1
+ properties("replication_num"="1")
+ """
+ sql "insert into uniform_agg_witness values (1, 7, null), (2, 7, 9)"
+
+ def nullableCountPlan = sql("""
+ explain select b, c, count(*) as n, sum(h) as sh
+ from (
+ select pk, b, count(v) as c, ndv(v) as h
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, c
+ """).collect { it[0] }.join("\n")
+ assertTrue((nullableCountPlan =~ /group by: b\[#\d+\], c\[#\d+\]/).find(),
+ "nullable COUNT must remain in the outer group
keys:\n${nullableCountPlan}")
+
+ def nullableNdvPlan = sql("""
+ explain select b, h, count(*) as n, sum(c) as sc
+ from (
+ select pk, b, count(v) as c, ndv(v) as h
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, h
+ """).collect { it[0] }.join("\n")
+ assertTrue((nullableNdvPlan =~ /group by: b\[#\d+\], h\[#\d+\]/).find(),
+ "nullable NDV must remain in the outer group
keys:\n${nullableNdvPlan}")
+
+ def nonNullableCountPlan = sql("""
+ explain select b, c, count(*) as n
+ from (
+ select pk, b, count(b) as c
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, c
+ """).collect { it[0] }.join("\n")
+ assertTrue((nonNullableCountPlan =~ /group by: b\[#\d+\]/).find(),
+ "non-null COUNT should keep the safe group-key
elimination:\n${nonNullableCountPlan}")
+ assertFalse((nonNullableCountPlan =~ /group by: b\[#\d+\],
c\[#\d+\]/).find(),
+ "non-null COUNT should not remain in the outer group
keys:\n${nonNullableCountPlan}")
+
+ order_qt_nullable_count_not_uniform """
+ select b, c, count(*) as n, sum(h) as sh
+ from (
+ select pk, b, count(v) as c, ndv(v) as h
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, c
+ order by b, c
+ """
+ order_qt_nullable_ndv_not_uniform """
+ select b, h, count(*) as n, sum(c) as sc
+ from (
+ select pk, b, count(v) as c, ndv(v) as h
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, h
+ order by b, h
+ """
+ order_qt_multi_argument_count_not_uniform """
+ select b, c, count(*) as n
+ from (
+ select pk, b, count(distinct b, v) as c
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, c
+ order by b, c
+ """
+ order_qt_non_nullable_count_uniform """
+ select b, c, count(*) as n
+ from (
+ select pk, b, count(b) as c
+ from uniform_agg_witness
+ group by pk, b
+ ) s
+ group by b, c
+ order by b, c
+ """
+
qt_project_const "select sum(c1), c2 from (select a c1,1 c2, d c3 from
eli_gbk_by_uniform_t) t group by c2,c3 order by 1,2;"
qt_project_slot_uniform "select max(c3), c1,c2,c3 from (select a c1,1 c2,
d c3 from eli_gbk_by_uniform_t where a=1) t group by c1,c2,c3 order by 1,2,3,4;"
@@ -332,4 +423,4 @@ GROUP BY
);
"""
-}
\ No newline at end of file
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
