This is an automated email from the ASF dual-hosted git repository.
yujun777 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 fabe0ee3ba6 [fix](mv rewrite) Return 0 for a COUNT rollup that hits a
materialized view (#68863)
fabe0ee3ba6 is described below
commit fabe0ee3ba60719e2a11a136c18452e303314724
Author: yujun <[email protected]>
AuthorDate: Mon Oct 12 09:57:47 2026 +0800
[fix](mv rewrite) Return 0 for a COUNT rollup that hits a materialized view
(#68863)
`Count.constructRollUp` rolled a non-DISTINCT count up to `sum`, so a
query whose
predicate filters out every row of the materialized view aggregated an
empty input.
`SUM` is NULL on empty input, while a scalar `COUNT` without `GROUP BY`
must return
one row of 0.
The rewritten output kept the nullability of the query's `COUNT`, which
is a
`NotNullableAggregateFunction`, so
`MaterializedViewUtils.normalizeExpression` wrapped
the rolled-up `sum` in `non_nullable` and the BE reported the NULL as a
hard error
instead of returning 0:
```sql
CREATE TABLE base (id INT, k INT) DUPLICATE KEY(id) DISTRIBUTED BY HASH(id)
BUCKETS 1
PROPERTIES ("replication_num" = "1");
INSERT INTO base VALUES (1,1),(2,1);
CREATE MATERIALIZED VIEW mv
BUILD DEFERRED REFRESH COMPLETE ON MANUAL
DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES ("replication_num" = "1")
AS SELECT k, COUNT(*) n FROM base GROUP BY k;
REFRESH MATERIALIZED VIEW mv COMPLETE; -- SUCCESS, mv holds (1, 2)
SELECT /*+ use_mv(mv) */ COUNT(*) FROM base WHERE k > 999;
-- before: errCode = 2, detailMessage = [INVALID_ARGUMENT]There's NULL
value in
-- column which is illegal for non_nullable
-- after: 0
-- the same query with enable_materialized_view_rewrite=false already
returned 0
```
Fix: roll a non-DISTINCT count up to `sum0`, which is also a
`NotNullableAggregateFunction` and returns 0 on empty input — exactly
the
`resultForEmptyInput()` contract `Count` already declares. Its
`ret(BigIntType).args(BigIntType)` signature matches the BIGINT count
column stored in
the MV, so the output slot of the rewrite does not change, and
`Sum0.constructRollUp`
returns `Sum0`, keeping a multi-level rollup self-consistent. A grouped
rollup is
unaffected because a group only appears when at least one MV row
survives, where `sum`
and `sum0` agree row by row. The DISTINCT branch already rolled up to
`BitmapUnionCount`, whose empty-input result is 0, so it stays as is.
---
.../nereids/rules/rewrite/SetPreAggStatus.java | 20 +++++
.../doris/nereids/stats/ExpressionEstimation.java | 7 ++
.../trees/expressions/functions/agg/Count.java | 3 +-
.../nereids/rules/rewrite/SetPreAggStatusTest.java | 6 ++
.../nereids/stats/ExpressionEstimationTest.java | 14 ++++
.../count_rollup_empty_input.out | 29 +++++++
.../count_rollup_empty_input.groovy | 92 ++++++++++++++++++++++
7 files changed, 170 insertions(+), 1 deletion(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
index 711b7b3fa08..d852f80357d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
@@ -40,6 +40,7 @@ import
org.apache.doris.nereids.trees.expressions.functions.agg.HllUnionAgg;
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.Sum0;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.Combinator;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.UnionCombinator;
@@ -758,6 +759,15 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
@Override
public PreAggStatus visitSum(Sum sum, AggregateType aggregateType)
{
+ return checkSumLike(sum, aggregateType);
+ }
+
+ @Override
+ public PreAggStatus visitSum0(Sum0 sum0, AggregateType
aggregateType) {
+ return checkSumLike(sum0, aggregateType);
+ }
+
+ private PreAggStatus checkSumLike(AggregateFunction sum,
AggregateType aggregateType) {
if (aggregateType == AggregateType.SUM && !sum.isDistinct()) {
return PreAggStatus.on();
} else {
@@ -860,6 +870,16 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
@Override
public PreAggStatus visitSum(Sum sum, List<Expression>
returnValues) {
+ return checkSumLike(sum, returnValues);
+ }
+
+ @Override
+ public PreAggStatus visitSum0(Sum0 sum0, List<Expression>
returnValues) {
+ // sum0 differs from sum only on an empty input, which
pre-aggregation cannot produce
+ return checkSumLike(sum0, returnValues);
+ }
+
+ private PreAggStatus checkSumLike(AggregateFunction sum,
List<Expression> returnValues) {
// DISTINCT breaks pre-agg: storage SUM merges duplicate full
keys
// first (e.g. two rowsets with v7=1 become 2), so
sum(DISTINCT ...)
// over the merged value differs from DISTINCT over the raw
rows.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java
index 265c7f68bdb..380fa421218 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/stats/ExpressionEstimation.java
@@ -44,6 +44,7 @@ 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.Sum0;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Abs;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Acos;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Ascii;
@@ -427,6 +428,12 @@ public class ExpressionEstimation extends
ExpressionVisitor<ColumnStatistic, Sta
return
ColumnStatistic.UNKNOWN.withAvgSizeByte(sum.getDataType().width());
}
+ // TODO: return a proper estimated stat after supports histogram
+ @Override
+ public ColumnStatistic visitSum0(Sum0 sum0, Statistics context) {
+ return
ColumnStatistic.UNKNOWN.withAvgSizeByte(sum0.getDataType().width());
+ }
+
// TODO: return a proper estimated stat after supports histogram
@Override
public ColumnStatistic visitAvg(Avg avg, Statistics context) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
index fa836721f52..ad41b6c20f3 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Count.java
@@ -188,7 +188,8 @@ public class Count extends NotNullableAggregateFunction
if (this.isDistinct()) {
return new BitmapUnionCount(param);
} else {
- return new Sum(param);
+ // sum0, not sum: count returns 0 on empty input while sum returns
NULL
+ return new Sum0(param);
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatusTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatusTest.java
index 737ce5fab17..9929e4bc3a4 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatusTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatusTest.java
@@ -32,6 +32,7 @@ import
org.apache.doris.nereids.trees.expressions.functions.agg.Max;
import org.apache.doris.nereids.trees.expressions.functions.agg.MaxBy;
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.Sum0;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.MergeCombinator;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.StateCombinator;
import
org.apache.doris.nereids.trees.expressions.functions.combinator.UnionCombinator;
@@ -254,12 +255,17 @@ class SetPreAggStatusTest {
// sum(v) with SUM-type value column (OneValueSlotAggChecker)
Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Sum(vSum)), Collections.emptySet(), output)
.isOn());
+ // sum0(v) is pre-aggregable like sum(v): the two differ only on an
empty input
+ Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Sum0(vSum)), Collections.emptySet(), output)
+ .isOn());
// max(v) with MAX-type value column
Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Max(vMax)), Collections.emptySet(),
Sets.newHashSet(vMax)).isOn());
// aggregation-type mismatch
Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Sum(vMax)), Collections.emptySet(),
Sets.newHashSet(vMax)).isOff());
+ Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Sum0(vMax)), Collections.emptySet(),
+ Sets.newHashSet(vMax)).isOff());
// count over a bare value column is not pre-aggregable
(OneValueSlotAggChecker has no visitCount)
Assertions.assertTrue(checkAggregateFunctions(Sets.newHashSet(new
Count(vSum)), Collections.emptySet(),
output).isOff());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/ExpressionEstimationTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/ExpressionEstimationTest.java
index 8082df696d1..00d2a7bc07b 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/ExpressionEstimationTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/ExpressionEstimationTest.java
@@ -30,6 +30,8 @@ import org.apache.doris.nereids.trees.expressions.Subtract;
import org.apache.doris.nereids.trees.expressions.WhenClause;
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.Sum0;
import org.apache.doris.nereids.trees.expressions.functions.scalar.If;
import org.apache.doris.nereids.trees.expressions.functions.scalar.NonNullable;
import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
@@ -40,6 +42,7 @@ import
org.apache.doris.nereids.trees.expressions.literal.DecimalLiteral;
import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral;
+import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.types.DateType;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.IntegerType;
@@ -417,6 +420,17 @@ class ExpressionEstimationTest {
Assertions.assertEquals(0, est.numNulls);
}
+ @Test
+ public void testSumAndSum0Width() {
+ Statistics stats = new Statistics(1000, new HashMap<>());
+ SlotReference value = new SlotReference("v", BigIntType.INSTANCE);
+
+ // a materialized view stores COUNT as a BIGINT column and the rollup
reads it back
+ // through sum0, so sum0 must be estimated as wide as the sum it
replaced
+ Assertions.assertEquals(8, ExpressionEstimation.estimate(new
Sum(value), stats).avgSizeByte);
+ Assertions.assertEquals(8, ExpressionEstimation.estimate(new
Sum0(value), stats).avgSizeByte);
+ }
+
@Test
public void testLiteral() {
Statistics stats = new Statistics(1000, new HashMap<>());
diff --git
a/regression-test/data/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.out
b/regression-test/data/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.out
new file mode 100644
index 00000000000..fd05aa94567
--- /dev/null
+++
b/regression-test/data/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.out
@@ -0,0 +1,29 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !mv_content --
+1 2
+2 1
+
+-- !rollup_empty_input --
+0
+
+-- !rollup_empty_input_without_mv --
+0
+
+-- !rollup_partial_input --
+2
+
+-- !rollup_grouped --
+1 2
+2 1
+
+-- !rollup_all --
+3
+
+-- !agg_preagg --
+1 3
+2 1
+
+-- !agg_preagg_sum --
+1 3
+2 1
+
diff --git
a/regression-test/suites/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.groovy
b/regression-test/suites/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.groovy
new file mode 100644
index 00000000000..15f3e011cc4
--- /dev/null
+++
b/regression-test/suites/nereids_rules_p0/mv/count_rollup_empty_input/count_rollup_empty_input.groovy
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("count_rollup_empty_input") {
+ String db = context.config.getDbNameByFile(context.file)
+ sql "use ${db}"
+ sql "set pre_materialized_view_rewrite_strategy = TRY_IN_RBO"
+ sql "set enable_sql_cache = false"
+
+ sql "DROP MATERIALIZED VIEW IF EXISTS rollup_empty_mv"
+ sql "DROP TABLE IF EXISTS rollup_empty_base"
+ sql "DROP TABLE IF EXISTS rollup_empty_agg"
+
+ sql """
+ CREATE TABLE rollup_empty_base (id INT, k INT)
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+
+ sql "INSERT INTO rollup_empty_base VALUES (1,1),(2,1),(3,2)"
+
+ sql """
+ CREATE MATERIALIZED VIEW rollup_empty_mv
+ BUILD DEFERRED REFRESH COMPLETE ON MANUAL
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ AS SELECT k, COUNT(*) n FROM rollup_empty_base GROUP BY k
+ """
+
+ sql "REFRESH MATERIALIZED VIEW rollup_empty_mv COMPLETE"
+ waitingMTMVTaskFinishedByMvName("rollup_empty_mv")
+
+ sql "ANALYZE TABLE rollup_empty_base WITH SYNC"
+ sql "ANALYZE TABLE rollup_empty_mv WITH SYNC"
+
+ order_qt_mv_content "SELECT k, n FROM rollup_empty_mv"
+
+ // The predicate filters out every row of the MV, but a count without
GROUP BY still has to
+ // output one row of 0, so the rolled up aggregate must not be a plain sum.
+ mv_rewrite_success("SELECT /*+ use_mv(rollup_empty_mv) */ COUNT(*) FROM
rollup_empty_base WHERE k > 999",
+ "rollup_empty_mv")
+ explain {
+ sql("SELECT /*+ use_mv(rollup_empty_mv) */ COUNT(*) FROM
rollup_empty_base WHERE k > 999")
+ contains "sum0"
+ notContains "non_nullable"
+ }
+ qt_rollup_empty_input "SELECT /*+ use_mv(rollup_empty_mv) */ COUNT(*) FROM
rollup_empty_base WHERE k > 999"
+
+ sql "SET enable_materialized_view_rewrite = false"
+ qt_rollup_empty_input_without_mv "SELECT COUNT(*) FROM rollup_empty_base
WHERE k > 999"
+ sql "SET enable_materialized_view_rewrite = true"
+
+ // Rows that do survive the filter keep the same counts as before.
+ qt_rollup_partial_input "SELECT /*+ use_mv(rollup_empty_mv) */ COUNT(*)
FROM rollup_empty_base WHERE k = 1"
+
+ // A grouped count rollup never sees an empty group, so its results are
unchanged either.
+ order_qt_rollup_grouped "SELECT /*+ use_mv(rollup_empty_mv) */ k, COUNT(*)
FROM rollup_empty_base GROUP BY k"
+
+ order_qt_rollup_all "SELECT /*+ use_mv(rollup_empty_mv) */ COUNT(*) FROM
rollup_empty_base"
+
+ // A synchronous aggregate materialized view keeps COUNT as a SUM value
column, so the rollup
+ // reading it back has to keep pushing pre-aggregation down to storage
instead of disabling it.
+ sql """
+ CREATE TABLE rollup_empty_agg (k INT, n BIGINT SUM DEFAULT "0")
+ AGGREGATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+ sql "INSERT INTO rollup_empty_agg VALUES (1, 2), (1, 1), (2, 1)"
+
+ explain {
+ sql("SELECT k, sum0(n) FROM rollup_empty_agg GROUP BY k")
+ contains "PREAGGREGATION: ON"
+ }
+ order_qt_agg_preagg "SELECT k, sum0(n) FROM rollup_empty_agg GROUP BY k"
+ order_qt_agg_preagg_sum "SELECT k, sum(n) FROM rollup_empty_agg GROUP BY k"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
