This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 3ebffe4224d186496d88b8d334d6997dac51fdf2 Author: seawinde <[email protected]> AuthorDate: Tue Feb 6 11:49:56 2024 +0800 [fix](nereids) Fix use aggregate mv wrongly when rewrite query which only contains join (#30858) the materialized view def is as following: > select > o_orderdate, > o_shippriority, > o_comment, > l_orderkey, > o_orderkey, > count(*) > from > orders > left join lineitem on l_orderkey = o_orderkey > group by o_orderdate, > o_shippriority, > o_comment, > l_orderkey; the query should rewrite success by using above materialized view > select > o_orderdate, > o_shippriority, > o_comment, > l_orderkey, > ps_partkey, > count(*) > from > orders left > join lineitem on l_orderkey = o_orderkey > left join partsupp on ps_partkey = l_orderkey > group by > o_orderdate, > o_shippriority, > o_comment, > l_orderkey, > ps_partkey; --- .../mv/AbstractMaterializedViewAggregateRule.java | 13 ++-- .../mv/AbstractMaterializedViewJoinRule.java | 8 ++- .../nereids/rules/exploration/mv/StructInfo.java | 76 ++++++++++++++++++---- .../mv/agg_with_roll_up/aggregate_with_roll_up.out | 14 ++++ .../agg_with_roll_up/aggregate_with_roll_up.groovy | 43 ++++++++++++ .../mv/dimension/dimension_2_5.groovy | 4 +- 6 files changed, 138 insertions(+), 20 deletions(-) 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 514469ac76e..24ecb41295b 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 @@ -18,6 +18,7 @@ package org.apache.doris.nereids.rules.exploration.mv; import org.apache.doris.common.Pair; +import org.apache.doris.nereids.rules.exploration.mv.StructInfo.PlanCheckContext; import org.apache.doris.nereids.rules.exploration.mv.StructInfo.PlanSplitContext; import org.apache.doris.nereids.rules.exploration.mv.mapping.SlotMapping; import org.apache.doris.nereids.trees.expressions.Alias; @@ -363,12 +364,16 @@ public abstract class AbstractMaterializedViewAggregateRule extends AbstractMate } } - // Check Aggregate is simple or not and check join is whether valid or not. - // Support project, filter, join, logical relation node and join condition should only contain - // slot reference equals currently. + /** + * Check Aggregate is simple or not and check join is whether valid or not. + * Support project, filter, join, logical relation node and join condition should only contain + * slot reference equals currently. + */ @Override protected boolean checkPattern(StructInfo structInfo) { - return structInfo.getTopPlan().accept(StructInfo.PLAN_PATTERN_CHECKER, SUPPORTED_JOIN_TYPE_SET); + PlanCheckContext checkContext = PlanCheckContext.of(SUPPORTED_JOIN_TYPE_SET); + return structInfo.getTopPlan().accept(StructInfo.PLAN_PATTERN_CHECKER, checkContext) + && checkContext.isContainsTopAggregate() && checkContext.getTopAggregateNum() <= 1; } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewJoinRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewJoinRule.java index 03549297bf8..dba0afb4925 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewJoinRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewJoinRule.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.rules.exploration.mv; import org.apache.doris.common.Pair; +import org.apache.doris.nereids.rules.exploration.mv.StructInfo.PlanCheckContext; import org.apache.doris.nereids.rules.exploration.mv.mapping.SlotMapping; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.Expression; @@ -76,10 +77,13 @@ public abstract class AbstractMaterializedViewJoinRule extends AbstractMateriali /** * Check join is whether valid or not. Support join's input only support project, filter, join, - * logical relation node and join condition should be slot reference equals currently + * logical relation, simple aggregate node. Con not have aggregate above on join. + * Join condition should be slot reference equals currently. */ @Override protected boolean checkPattern(StructInfo structInfo) { - return structInfo.getTopPlan().accept(StructInfo.PLAN_PATTERN_CHECKER, SUPPORTED_JOIN_TYPE_SET); + PlanCheckContext checkContext = PlanCheckContext.of(SUPPORTED_JOIN_TYPE_SET); + return structInfo.getTopPlan().accept(StructInfo.PLAN_PATTERN_CHECKER, checkContext) + && !checkContext.isContainsTopAggregate(); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/StructInfo.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/StructInfo.java index af874d37b11..674d4935594 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/StructInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/StructInfo.java @@ -446,39 +446,91 @@ public class StructInfo { } } + /** + * The context for plan check context, make sure that the plan in query and mv is valid or not + */ + public static class PlanCheckContext { + // the aggregate above join + private boolean containsTopAggregate = false; + private int topAggregateNum = 0; + private boolean alreadyMeetJoin = false; + private final Set<JoinType> supportJoinTypes; + + public PlanCheckContext(Set<JoinType> supportJoinTypes) { + this.supportJoinTypes = supportJoinTypes; + } + + public boolean isContainsTopAggregate() { + return containsTopAggregate; + } + + public void setContainsTopAggregate(boolean containsTopAggregate) { + this.containsTopAggregate = containsTopAggregate; + } + + public boolean isAlreadyMeetJoin() { + return alreadyMeetJoin; + } + + public void setAlreadyMeetJoin(boolean alreadyMeetJoin) { + this.alreadyMeetJoin = alreadyMeetJoin; + } + + public Set<JoinType> getSupportJoinTypes() { + return supportJoinTypes; + } + + public int getTopAggregateNum() { + return topAggregateNum; + } + + public void plusTopAggregateNum() { + this.topAggregateNum += 1; + } + + public static PlanCheckContext of(Set<JoinType> supportJoinTypes) { + return new PlanCheckContext(supportJoinTypes); + } + } + /** * PlanPatternChecker, this is used to check the plan pattern is valid or not */ - public static class PlanPatternChecker extends DefaultPlanVisitor<Boolean, Set<JoinType>> { + public static class PlanPatternChecker extends DefaultPlanVisitor<Boolean, PlanCheckContext> { @Override public Boolean visitLogicalJoin(LogicalJoin<? extends Plan, ? extends Plan> join, - Set<JoinType> supportJoinTypes) { - if (!supportJoinTypes.contains(join.getJoinType())) { + PlanCheckContext checkContext) { + checkContext.setAlreadyMeetJoin(true); + if (!checkContext.getSupportJoinTypes().contains(join.getJoinType())) { return false; } if (!join.getOtherJoinConjuncts().isEmpty()) { return false; } - return visit(join, supportJoinTypes); + return visit(join, checkContext); } @Override public Boolean visitLogicalAggregate(LogicalAggregate<? extends Plan> aggregate, - Set<JoinType> supportJoinTypes) { + PlanCheckContext checkContext) { + if (!checkContext.isAlreadyMeetJoin()) { + checkContext.setContainsTopAggregate(true); + checkContext.plusTopAggregateNum(); + } if (aggregate.getSourceRepeat().isPresent()) { return false; } - return visit(aggregate, supportJoinTypes); + return visit(aggregate, checkContext); } @Override - public Boolean visitGroupPlan(GroupPlan groupPlan, Set<JoinType> supportJoinTypes) { + public Boolean visitGroupPlan(GroupPlan groupPlan, PlanCheckContext checkContext) { return groupPlan.getGroup().getLogicalExpressions().stream() - .anyMatch(logicalExpression -> logicalExpression.getPlan().accept(this, supportJoinTypes)); + .anyMatch(logicalExpression -> logicalExpression.getPlan().accept(this, checkContext)); } @Override - public Boolean visit(Plan plan, Set<JoinType> supportJoinTypes) { + public Boolean visit(Plan plan, PlanCheckContext checkContext) { if (plan instanceof Filter || plan instanceof Project || plan instanceof CatalogRelation @@ -486,14 +538,14 @@ public class StructInfo { || plan instanceof LogicalSort || plan instanceof LogicalAggregate || plan instanceof GroupPlan) { - return doVisit(plan, supportJoinTypes); + return doVisit(plan, checkContext); } return false; } - private Boolean doVisit(Plan plan, Set<JoinType> supportJoinTypes) { + private Boolean doVisit(Plan plan, PlanCheckContext checkContext) { for (Plan child : plan.children()) { - boolean valid = child.accept(this, supportJoinTypes); + boolean valid = child.accept(this, checkContext); if (!valid) { return false; } 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 c1b81931f16..8a1f3fb70d9 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 @@ -299,3 +299,17 @@ 4 4 68 100.0000 36.5000 6 1 0 22.0000 57.2000 +-- !query31_0_before -- +2023-12-08 1 yy 1 \N 2 +2023-12-09 1 yy 2 2 2 +2023-12-10 1 yy 3 \N 2 +2023-12-11 2 mm 4 \N 1 +2023-12-12 2 mi 5 \N 2 + +-- !query31_0_after -- +2023-12-08 1 yy 1 \N 2 +2023-12-09 1 yy 2 2 2 +2023-12-10 1 yy 3 \N 2 +2023-12-11 2 mm 4 \N 1 +2023-12-12 2 mi 5 \N 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 bc2d9f13525..0b0c4aa107b 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 @@ -1225,4 +1225,47 @@ suite("aggregate_with_roll_up") { check_mv_rewrite_fail(db, mv30_0, query30_0, "mv30_0") order_qt_query30_0_after "${query30_0}" sql """ DROP MATERIALIZED VIEW IF EXISTS mv30_0""" + + // should rewrite fail, because the part of query is join but mv is aggregate + def mv31_0 = """ + select + o_orderdate, + o_shippriority, + o_comment, + l_orderkey, + o_orderkey, + count(*) + from + orders + left join lineitem on l_orderkey = o_orderkey + group by o_orderdate, + o_shippriority, + o_comment, + l_orderkey, + o_orderkey; + """ + def query31_0 = """ + select + o_orderdate, + o_shippriority, + o_comment, + l_orderkey, + ps_partkey, + count(*) + from + orders left + join lineitem on l_orderkey = o_orderkey + left join partsupp on ps_partkey = l_orderkey + group by + o_orderdate, + o_shippriority, + o_comment, + l_orderkey, + ps_partkey; + """ + order_qt_query31_0_before "${query31_0}" + check_mv_rewrite_fail(db, mv31_0, query31_0, "mv31_0") + order_qt_query31_0_after "${query31_0}" + sql """ DROP MATERIALIZED VIEW IF EXISTS mv31_0""" + } diff --git a/regression-test/suites/nereids_rules_p0/mv/dimension/dimension_2_5.groovy b/regression-test/suites/nereids_rules_p0/mv/dimension/dimension_2_5.groovy index 762408ba057..6e6bb4bccdf 100644 --- a/regression-test/suites/nereids_rules_p0/mv/dimension/dimension_2_5.groovy +++ b/regression-test/suites/nereids_rules_p0/mv/dimension/dimension_2_5.groovy @@ -304,7 +304,7 @@ suite("partition_mv_rewrite_dimension_2_5") { o_comment """ explain { sql("${sql_stmt_5}") - contains "${mv_name_5}(${mv_name_5})" + notContains "${mv_name_5}(${mv_name_5})" } compare_res(sql_stmt_5 + " order by 1,2,3") @@ -318,7 +318,7 @@ suite("partition_mv_rewrite_dimension_2_5") { o_comment """ explain { sql("${sql_stmt_5_2}") - contains "${mv_name_5}(${mv_name_5})" + notContains "${mv_name_5}(${mv_name_5})" } compare_res(sql_stmt_5_2 + " order by 1,2,3") sql """DROP MATERIALIZED VIEW IF EXISTS ${mv_name_5};""" --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
