yujun777 commented on code in PR #68861:
URL: https://github.com/apache/doris/pull/68861#discussion_r4278144530
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java:
##########
@@ -297,13 +297,14 @@ public static PredicateCompensation compensatePredicates(
}
Set<Expression> queryBasedViewResidualPredicates =
collectNonInferredQueryBasedExpressions(
viewStructInfo.getSplitPredicate().getResidualPredicateMap().keySet(),
viewToQuerySlotMapping);
- Set<Expression> exactCoveredPredicates = new LinkedHashSet<>();
- exactCoveredPredicates.addAll(Sets.intersection(
- compensationCandidates.getEquals().keySet(),
queryBasedViewResidualPredicates));
- exactCoveredPredicates.addAll(Sets.intersection(
- compensationCandidates.getRanges().keySet(),
queryBasedViewResidualPredicates));
- exactCoveredPredicates.addAll(Sets.intersection(
- compensationCandidates.getResiduals().keySet(),
queryBasedViewResidualPredicates));
+ // Only a candidate the view enforces over the same value is
redundant. A candidate which constrains
+ // an aggregate or window output is not, even when the view states the
identical condition: the view
+ // enforced it over the rows it materialized, the query needs it over
the value its own aggregate
+ // produces from those rows, and a roll up changes that value, an
aggregate over no row at all. Keep
+ // such candidates, so that the check at the end refuses the rewrite
instead of dropping the filter.
+ Set<Expression> prunableCandidates =
collectPrunableCandidates(compensationCandidates);
Review Comment:
Convinced — narrowed in 8b80826699c. The candidate is only kept out of the
pruning when the rewrite computes that value again.
`Predicates.compensatePredicates` takes an `aggregateOutputRecomputed` flag
which `AbstractMaterializedViewRule.predicatesCompensate` fills from a new
`aggregateOutputIsRecomputed` hook; the hook defaults to false and the
aggregate rule sets it for exactly the shape `isGroupByEquals` refuses to
eliminate — a query without grouping over a view which groups by something.
Every other shape prunes the covered predicate as before.
The match you describe is rewritten by the mv again, with
`enable_sql_cache=false`:
```
mv and query: select k, count(*) c from base group by k having count(*) > 10
memo: gmv chose
shape: PhysicalResultSink|--PhysicalProject|----PhysicalOlapScan[gmv]
rows: 100 with the rewrite on, 100 with
enable_materialized_view_rewrite=false
```
and `PredicatesTest` is green again — 15 tests,
`testCoveredWindowResidualIsAllowedAfterFinalization` among them; that test is
what caught this, so the static read was right.
The unsafe roll up stays refused: for `select count(*) from base where k = 1
having count(*) > {0,5,19,20,21,100000}` and the same six with `k = 9999`,
against the `group by k having count(*) > 0` mv, all twelve answers match the
rewrite-disabled one and the mv is not chosen. `aggregate_with_roll_up`,
`window_above_aggregate`, `null_un_safe_equals` and `mv_ignore_predicate`
re-pass with the final code.
##########
regression-test/suites/nereids_rules_p0/mv/agg_scalar_empty_input/agg_scalar_empty_input.groovy:
##########
@@ -0,0 +1,168 @@
+// 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("agg_scalar_empty_input") {
+ String db = context.config.getDbNameByFile(context.file)
+ sql "use ${db}"
+ sql "set pre_materialized_view_rewrite_strategy = TRY_IN_RBO"
+ sql "set runtime_filter_mode=OFF"
+ sql "SET ignore_shape_nodes='PhysicalDistribute'"
+
+ sql """DROP MATERIALIZED VIEW IF EXISTS scalar_agg_empty_mv"""
+ sql """DROP MATERIALIZED VIEW IF EXISTS scalar_agg_uniform_mv"""
+ sql """DROP TABLE IF EXISTS scalar_agg_empty_base"""
+
+ sql """
+ CREATE TABLE scalar_agg_empty_base (k INT NOT NULL, v INT NOT NULL)
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+
+ // 2000 rows over 100 distinct k values, so the pre-aggregated mv is much
cheaper to read than
+ // the base table and the rewritten plan is the one the optimizer picks. k
= 9999 matches nothing,
+ // and k = 1 holds v = 1, 101, ... 1901.
+ sql """INSERT INTO scalar_agg_empty_base SELECT number % 100, number FROM
numbers("number" = "2000")"""
+ sql """ANALYZE TABLE scalar_agg_empty_base WITH SYNC"""
+
+ def mvSql = """select k, count(*) as cnt, sum(v) as s, min(v) as mn,
max(v) as mx """ +
+ """from scalar_agg_empty_base group by k"""
+
+ def qCountEmpty = """select count(*) from scalar_agg_empty_base where k =
9999"""
+ def qCountMatched = """select count(*) from scalar_agg_empty_base where k
= 1"""
+ def qCountAll = """select count(*) from scalar_agg_empty_base"""
+ def qCountProjected = """select count(*) + 1 from scalar_agg_empty_base
where k = 9999"""
+ def qCountDistinct = """select count(distinct v) from
scalar_agg_empty_base where k = 9999"""
+ def qSumEmpty = """select sum(v) from scalar_agg_empty_base where k =
9999"""
+ def qSumMatched = """select sum(v) from scalar_agg_empty_base where k =
1"""
+ def qSumAll = """select sum(v) from scalar_agg_empty_base"""
+ def qMinEmpty = """select min(v) from scalar_agg_empty_base where k =
9999"""
+ def qMinMatched = """select min(v) from scalar_agg_empty_base where k =
1"""
+ def qMaxEmpty = """select max(v) from scalar_agg_empty_base where k =
9999"""
+ def qMaxMatched = """select max(v) from scalar_agg_empty_base where k =
1"""
+
+ // Reference results, computed on the base table: a scalar aggregate
always emits exactly one
+ // row, COUNT of an empty input is 0 while SUM/MIN/MAX of an empty input
are NULL.
+ sql "set enable_materialized_view_rewrite=false"
+ order_qt_count_off_empty "${qCountEmpty}"
+ order_qt_count_off_matched "${qCountMatched}"
+ order_qt_count_off_all "${qCountAll}"
+ order_qt_count_off_projected "${qCountProjected}"
+ order_qt_count_off_distinct "${qCountDistinct}"
+ order_qt_sum_off_empty "${qSumEmpty}"
+ order_qt_sum_off_matched "${qSumMatched}"
+ order_qt_sum_off_all "${qSumAll}"
+ order_qt_min_off_empty "${qMinEmpty}"
+ order_qt_min_off_matched "${qMinMatched}"
+ order_qt_max_off_empty "${qMaxEmpty}"
+ order_qt_max_off_matched "${qMaxMatched}"
+
+ // The mv groups by k while these queries are scalar, so the mv dimensions
may only be dropped
+ // if the aggregate node is kept: rolling COUNT up must yield 0 and
SUM/MIN/MAX must yield NULL.
+ sql "set enable_materialized_view_rewrite=true"
+ async_mv_rewrite_success(db, mvSql, qCountEmpty, "scalar_agg_empty_mv")
+ sql """ANALYZE TABLE scalar_agg_empty_mv WITH SYNC"""
+ mv_rewrite_success(qCountMatched, "scalar_agg_empty_mv")
+ mv_rewrite_success(qCountProjected, "scalar_agg_empty_mv")
+ mv_rewrite_success(qSumEmpty, "scalar_agg_empty_mv")
+ mv_rewrite_success(qSumMatched, "scalar_agg_empty_mv")
+ mv_rewrite_success(qSumAll, "scalar_agg_empty_mv")
+ mv_rewrite_success(qMinEmpty, "scalar_agg_empty_mv")
+ mv_rewrite_success(qMinMatched, "scalar_agg_empty_mv")
+ mv_rewrite_success(qMaxEmpty, "scalar_agg_empty_mv")
+ mv_rewrite_success(qMaxMatched, "scalar_agg_empty_mv")
+ // count(distinct v) is stored in the mv as an already computed value and
can not be rolled up,
+ // so the mv must not answer this query at all.
+ mv_rewrite_fail(qCountDistinct, "scalar_agg_empty_mv")
+ // count(*) without a predicate is answered straight from the FE table
metadata by
+ // REWRITE_SIMPLE_AGG_TO_CONSTANT, so the mv is not expected to be used
for it.
+
+ // The scalar aggregate must survive the rewrite: an mv scan alone would
return no row at all.
+ order_qt_count_on_empty "${qCountEmpty}"
+ order_qt_count_on_matched "${qCountMatched}"
+ order_qt_count_on_all "${qCountAll}"
+ order_qt_count_on_projected "${qCountProjected}"
+ order_qt_count_on_distinct "${qCountDistinct}"
+ order_qt_sum_on_empty "${qSumEmpty}"
+ order_qt_sum_on_matched "${qSumMatched}"
+ order_qt_sum_on_all "${qSumAll}"
+ order_qt_min_on_empty "${qMinEmpty}"
+ order_qt_min_on_matched "${qMinMatched}"
+ order_qt_max_on_empty "${qMaxEmpty}"
+ order_qt_max_on_matched "${qMaxMatched}"
+ // Whether the scalar aggregate is split into a local and a global
aggregate is a cost based
+ // choice which depends on the number of backends, so pin the one phase
aggregation here to keep
+ // the shape assertion about the aggregate itself independent of the
cluster it runs on.
+ sql "set agg_phase=1"
+ qt_shape_count_on """explain shape plan ${qCountEmpty}"""
+ qt_shape_min_on """explain shape plan ${qMinEmpty}"""
+
+ // A having clause filters the rows the top aggregate produces. Keeping
the aggregate is what makes the
+ // roll up possible, and the filter then stays above it and is evaluated
on the rolled up value, so the
+ // mv is still used and a having which the single rolled up row does not
pass yields no row at all.
+ def qHavingPasses = """select count(*) from scalar_agg_empty_base where k
= 1 having count(*) > 0"""
+ def qHavingRejects = """select count(*) from scalar_agg_empty_base where k
= 1 having count(*) > 100000"""
+ def qHavingEmptyInput = """select count(*) from scalar_agg_empty_base
where k = 9999 having count(*) > 0"""
+ sql "set enable_materialized_view_rewrite=false"
+ order_qt_having_off_passes "${qHavingPasses}"
+ order_qt_having_off_rejects "${qHavingRejects}"
+ order_qt_having_off_empty "${qHavingEmptyInput}"
+ sql "set enable_materialized_view_rewrite=true"
+ mv_rewrite_success(qHavingPasses, "scalar_agg_empty_mv")
+ mv_rewrite_success(qHavingRejects, "scalar_agg_empty_mv")
+ mv_rewrite_success(qHavingEmptyInput, "scalar_agg_empty_mv")
+ order_qt_having_on_passes "${qHavingPasses}"
Review Comment:
Agreed — fixed in 8b80826699c: `SET enable_sql_cache=false` is now in the
suite header next to the other session settings, before the reference block, so
the rewrite-off result of `qHavingPasses` and its siblings can not be replayed
for the rewrite-on run. The suite passes with the cache disabled and its golden
file is unchanged, the `qt_shape_*` assertions included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
