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
commit 80583d60057d5e7c43d6884f7c0a6f33f1f9408d Author: yujun <[email protected]> AuthorDate: Thu Oct 15 09:25:18 2026 +0800 branch-4.1: [fix](mv rewrite) Return 0 for a COUNT rollup that hits a materialized view #68863 (#69072) cherry-pick: #68863 --- .../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 f81b6159200..6dbecd26293 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.scalar.GroupingScalarFunction; import org.apache.doris.nereids.trees.expressions.functions.scalar.If; import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; @@ -729,6 +730,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 { @@ -831,6 +841,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 0cdc906db07..cbc97701f77 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 @@ -46,6 +46,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; @@ -429,6 +430,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 b8c7a48242a..0396b13ce98 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 @@ -196,7 +196,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 aa93a831d07..7b39ed22de9 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 @@ -31,6 +31,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.If; import org.apache.doris.nereids.trees.expressions.functions.scalar.Random; import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral; @@ -203,12 +204,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 db3ca58ee70..8a2b405c553 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]
