github-actions[bot] commented on code in PR #68861: URL: https://github.com/apache/doris/pull/68861#discussion_r4270693428
########## 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: [P2] Disable SQL caching for the new HAVING result comparisons These new HAVING checks run the same SQL with MV rewrite off and on while leaving the default `enable_sql_cache=true`. If the suite has passed the default 30-second quiet window before the reference block, `qHavingPasses` at line 121 can cache the base-table result of 20. No write or DDL occurs before this rewrite-on execution, and toggling `enable_materialized_view_rewrite` changes neither the cache result-variable signature nor the base-table versions, so this query can replay that cached reference. The preceding `mv_rewrite_success` checks still inspect a fresh MEMO plan, but the result comparison then does not execute the selected MV rewrite. Set `enable_sql_cache=false` before the reference queries so the new HAVING result pairs validate the rewrite itself. This is a separate execution gap from the earlier empty-COUNT candidate-selection issue. ########## 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: [P2] Keep exact-covered predicates for value-preserving rewrites For an MV and query both `SELECT k, COUNT(*) c FROM t GROUP BY k HAVING COUNT(*) > 10`, the input is `Filter(c > 10) -> Aggregate([k], [k, COUNT(*) AS c]) -> Scan(t)` and the valid replacement is `Project(mv.k, mv.c) -> MVScan`: no aggregate value is recomputed. This exclusion keeps `COUNT(*) > 10` in the residual candidates; `removePredicatesImpliedByViewResidual` keeps it too, so `rejectUnsafeResidualCompensation` returns null before `isGroupByEquals` can select that direct rewrite. Matching just the aggregate child cannot recover it because that query lacks the view HAVING. Consequently even an exact grouped MV definition now falls back to base-table aggregation. The same blanket exclusion also contradicts `PredicatesTest.testCoveredWindowResidualIsAllowedAfterFinalization`, which expects empty nonnull compensation for identical QUALIFY predicates (static inspection; test not run). Make pruning depend on whether aggregate/window values are recomputed, retaining rejection for th e unsafe scalar rollup while allowing value-preserving matches. -- 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]
