From baf9a45e57cb32af9b0fcc44a36dee7a598b0dd0 Mon Sep 17 00:00:00 2001 From: SWEI Date: Wed, 15 May 2024 12:00:29 +0800 Subject: [PATCH] [fix](mtmv) check groupby in agg-bottom-plan when rewrite agg query by mv (#34274) check groupby in agg-bottom-plan when rewrite and rollup agg query by mv --- ...AbstractMaterializedViewAggregateRule.java | 26 +++++++++++++++++++ .../aggregate_with_roll_up.out | 14 ++++++++++ .../aggregate_with_roll_up.groovy | 25 ++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java index 5c8c479986..cb11e06150 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java @@ -230,6 +230,7 @@ public abstract class AbstractMaterializedViewAggregateRule extends AbstractMate // split the query top plan expressions to group expressions and functions, if can not, bail out. Pair, Set> queryGroupAndFunctionPair = topPlanSplitToGroupAndFunction(queryTopPlanAndAggPair, queryStructInfo); + Set queryTopPlanGroupBySet = queryGroupAndFunctionPair.key(); Set queryTopPlanFunctionSet = queryGroupAndFunctionPair.value(); // try to rewrite, contains both roll up aggregate functions and aggregate group expression List finalOutputExpressions = new ArrayList<>(); @@ -284,6 +285,31 @@ public abstract class AbstractMaterializedViewAggregateRule extends AbstractMate } // add project to guarantee group by column ref is slot reference, // this is necessary because physical createHash will need slotReference later + if (queryGroupByExpressions.size() != queryTopPlanGroupBySet.size()) { + for (Expression expression : queryGroupByExpressions) { + if (queryTopPlanGroupBySet.contains(expression)) { + continue; + } + Expression queryGroupShuttledExpr = ExpressionUtils.shuttleExpressionWithLineage( + expression, queryTopPlan, queryStructInfo.getTableBitSet()); + AggregateExpressionRewriteContext context = new AggregateExpressionRewriteContext(true, + mvExprToMvScanExprQueryBased, queryTopPlan, queryStructInfo.getTableBitSet()); + // group by expression maybe group by a + b, so we need expression rewriter + Expression rewrittenGroupByExpression = queryGroupShuttledExpr.accept(AGGREGATE_EXPRESSION_REWRITER, + context); + if (!context.isValid()) { + // group expr can not rewrite by view + materializationContext.recordFailReason(queryStructInfo, + "View dimensions doesn't not cover the query dimensions in bottom agg ", + () -> String.format("mvExprToMvScanExprQueryBased is %s,\n queryGroupShuttledExpr is %s", + mvExprToMvScanExprQueryBased, queryGroupShuttledExpr)); + return null; + } + NamedExpression groupByExpression = rewrittenGroupByExpression instanceof NamedExpression + ? (NamedExpression) rewrittenGroupByExpression : new Alias(rewrittenGroupByExpression); + finalGroupExpressions.add(groupByExpression); + } + } List copiedFinalGroupExpressions = new ArrayList<>(finalGroupExpressions); List projectsUnderAggregate = copiedFinalGroupExpressions.stream() .map(NamedExpression.class::cast) diff --git a/regression-test/data/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.out b/regression-test/data/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.out index b95c352191..cebddff983 100644 --- a/regression-test/data/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.out +++ b/regression-test/data/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.out @@ -341,3 +341,17 @@ 2023-12-11 2 mm 4 \N 1 2023-12-12 2 mi 5 \N 2 +-- !query32_0_before -- +2023-12-08 2 +2023-12-09 1 +2023-12-10 2 +2023-12-11 1 +2023-12-12 2 + +-- !query32_0_after -- +2023-12-08 2 +2023-12-09 1 +2023-12-10 2 +2023-12-11 1 +2023-12-12 2 + diff --git a/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy b/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy index d8037301c0..8664607be4 100644 --- a/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy +++ b/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy @@ -1360,4 +1360,29 @@ suite("aggregate_with_roll_up") { order_qt_query31_0_after "${query31_0}" sql """ DROP MATERIALIZED VIEW IF EXISTS mv31_0""" + // should rewrite fail, because the part of query is join but mv is aggregate + def mv32_0 = """ + select + o_orderdate, + count(*) + from + orders + group by + o_orderdate; + """ + def query32_0 = """ + select + o_orderdate, + count(*) + from + orders + group by + o_orderdate, + o_shippriority; + """ + order_qt_query32_0_before "${query32_0}" + check_mv_rewrite_fail(db, mv32_0, query32_0, "mv32_0") + order_qt_query32_0_after "${query32_0}" + sql """ DROP MATERIALIZED VIEW IF EXISTS mv32_0""" + }