From 5b8e5ecc24e674ca7bfbe5459b8658f80c708930 Mon Sep 17 00:00:00 2001 From: seawinde <149132972+seawinde@users.noreply.github.com> Date: Fri, 5 Jul 2024 11:45:00 +0800 Subject: [PATCH] [improvement](mtmv) improve mv rewrite performance by reuse the shuttled expression (#37197) Optimizations: 1. Expression shuttle is expensive in materialized view rewritting, So reuse the shuttled expression. 2. Generate shuttledExpressions by planOutput is also expensive, so generate and store in struct info once and used later --- .../java/org/apache/doris/mtmv/MTMVCache.java | 2 +- .../mv/AbstractMaterializedViewRule.java | 4 +- .../mv/AsyncMaterializationContext.java | 6 +- .../mv/MaterializationContext.java | 34 ++++---- .../exploration/mv/MaterializedViewUtils.java | 8 +- .../rules/exploration/mv/Predicates.java | 6 -- .../rules/exploration/mv/StructInfo.java | 83 ++++++++++++------- .../exploration/mv/HyperGraphAggTest.java | 4 +- .../doris/nereids/sqltest/SqlTestBase.java | 4 +- 9 files changed, 88 insertions(+), 63 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java b/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java index d0d66d187a57b8..a6403b5892e243 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCache.java @@ -108,7 +108,7 @@ public Plan visitLogicalResultSink(LogicalResultSink logicalResu return childContext.getRewritePlan(); }, mvPlan, originPlan); // Construct structInfo once for use later - Optional structInfoOptional = MaterializationContext.constructStructInfo(mvPlan, + Optional structInfoOptional = MaterializationContext.constructStructInfo(mvPlan, originPlan, planner.getCascadesContext(), new BitSet()); return new MTMVCache(mvPlan, originPlan, planner.getCascadesContext().getMemo().getRoot().getStatistics(), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java index cb1f796e3ff21f..fb82d065f3069f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java @@ -145,8 +145,8 @@ protected List getValidQueryStructInfos(Plan queryPlan, CascadesCont BitSet materializedViewTableSet) { List validStructInfos = new ArrayList<>(); // For every materialized view we should trigger refreshing struct info map - List uncheckedStructInfos = MaterializedViewUtils.extractStructInfo(queryPlan, cascadesContext, - materializedViewTableSet); + List uncheckedStructInfos = MaterializedViewUtils.extractStructInfo(queryPlan, queryPlan, + cascadesContext, materializedViewTableSet); uncheckedStructInfos.forEach(queryStructInfo -> { boolean valid = checkQueryPattern(queryStructInfo, cascadesContext) && queryStructInfo.isValid(); if (!valid) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AsyncMaterializationContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AsyncMaterializationContext.java index d830e9d41cbdee..eef0a36d301a20 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AsyncMaterializationContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AsyncMaterializationContext.java @@ -50,6 +50,7 @@ public class AsyncMaterializationContext extends MaterializationContext { private static final Logger LOG = LogManager.getLogger(AsyncMaterializationContext.class); private final MTMV mtmv; + private List materializationQualifier; /** * MaterializationContext, this contains necessary info for query rewriting by mv @@ -72,7 +73,10 @@ Plan doGenerateScanPlan(CascadesContext cascadesContext) { @Override List getMaterializationQualifier() { - return this.mtmv.getFullQualifiers(); + if (this.materializationQualifier == null) { + this.materializationQualifier = this.mtmv.getFullQualifiers(); + } + return this.materializationQualifier; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java index a383f9e19c4b3f..50f8a204cbc578 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializationContext.java @@ -36,7 +36,6 @@ import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan; import org.apache.doris.nereids.trees.plans.physical.PhysicalRelation; import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanVisitor; -import org.apache.doris.nereids.util.ExpressionUtils; import org.apache.doris.statistics.ColumnStatistic; import org.apache.doris.statistics.Statistics; @@ -119,32 +118,31 @@ public MaterializationContext(Plan plan, Plan originalPlan, Plan scanPlan, this.exprToScanExprMapping.put(originalPlanOutput.get(slotIndex), scanPlanOutput.get(slotIndex)); } } - this.planOutputShuttledExpressions = ExpressionUtils.shuttleExpressionWithLineage(originalPlanOutput, - originalPlan, new BitSet()); - // materialization output expression shuttle, this will be used to expression rewrite - this.shuttledExprToScanExprMapping = ExpressionMapping.generate( - this.planOutputShuttledExpressions, - scanPlanOutput); // Construct materialization struct info, catch exception which may cause planner roll back - if (structInfo == null) { - Optional structInfoOptional = constructStructInfo(plan, cascadesContext, new BitSet()); - if (!structInfoOptional.isPresent()) { - this.available = false; - } - this.structInfo = structInfoOptional.orElseGet(() -> null); - } else { - this.structInfo = structInfo; + this.structInfo = structInfo == null + ? constructStructInfo(plan, originalPlan, cascadesContext, new BitSet()).orElseGet(() -> null) + : structInfo; + this.available = this.structInfo != null; + if (available) { + this.planOutputShuttledExpressions = this.structInfo.getPlanOutputShuttledExpressions(); + // materialization output expression shuttle, this will be used to expression rewrite + this.shuttledExprToScanExprMapping = ExpressionMapping.generate( + this.planOutputShuttledExpressions, + scanPlanOutput); } } /** * Construct materialized view Struct info + * @param plan maybe remove unnecessary plan node, and the logical output maybe wrong + * @param originalPlan original plan, the output is right */ - public static Optional constructStructInfo(Plan plan, CascadesContext cascadesContext, - BitSet expectedTableBitSet) { + public static Optional constructStructInfo(Plan plan, Plan originalPlan, + CascadesContext cascadesContext, BitSet expectedTableBitSet) { List viewStructInfos; try { - viewStructInfos = MaterializedViewUtils.extractStructInfo(plan, cascadesContext, expectedTableBitSet); + viewStructInfos = MaterializedViewUtils.extractStructInfo(plan, originalPlan, + cascadesContext, expectedTableBitSet); if (viewStructInfos.size() > 1) { // view struct info should only have one, log error and use the first struct info LOG.warn(String.format("view strut info is more than one, materialization plan is %s", diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java index 0d4cb0a47c41c4..1a487791398a50 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java @@ -181,8 +181,10 @@ public static boolean containTableQueryOperator(Plan analyzedPlan) { /** * Extract struct info from plan, support to get struct info from logical plan or plan in group. + * @param plan maybe remove unnecessary plan node, and the logical output maybe wrong + * @param originalPlan original plan, the output is right */ - public static List extractStructInfo(Plan plan, CascadesContext cascadesContext, + public static List extractStructInfo(Plan plan, Plan originalPlan, CascadesContext cascadesContext, BitSet materializedViewTableSet) { // If plan belong to some group, construct it with group struct info if (plan.getGroupExpression().isPresent()) { @@ -202,7 +204,7 @@ public static List extractStructInfo(Plan plan, CascadesContext casc continue; } StructInfo structInfo = structInfoMap.getStructInfo(cascadesContext, - queryTableSet, ownerGroup, plan); + queryTableSet, ownerGroup, originalPlan); if (structInfo != null) { structInfosBuilder.add(structInfo); } @@ -211,7 +213,7 @@ public static List extractStructInfo(Plan plan, CascadesContext casc } } // if plan doesn't belong to any group, construct it directly - return ImmutableList.of(StructInfo.of(plan, cascadesContext)); + return ImmutableList.of(StructInfo.of(plan, originalPlan, cascadesContext)); } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java index 139230be5d4b97..8475bc6b46a157 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/Predicates.java @@ -41,7 +41,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; /** * This record the predicates which can be pulled up or some other type predicates. @@ -70,11 +69,6 @@ public Predicates merge(Collection predicates) { return new Predicates(mergedPredicates); } - public Expression composedExpression() { - return ExpressionUtils.and(pulledUpPredicates.stream().map(Expression.class::cast) - .collect(Collectors.toList())); - } - /** * Split the expression to equal, range and residual predicate. */ 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 d8fcf4a2c5378a..eeb2192565359a 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 @@ -106,15 +106,16 @@ public class StructInfo { // this is for LogicalCompatibilityContext later private final Map relationIdStructInfoNodeMap; // this recorde the predicates which can pull up, not shuttled - private Predicates predicates; + private final Predicates predicates; // split predicates is shuttled - private final SplitPredicate splitPredicate; - private final EquivalenceClass equivalenceClass; + private SplitPredicate splitPredicate; + private EquivalenceClass equivalenceClass; // Key is the expression shuttled and the value is the origin expression // this is for building LogicalCompatibilityContext later. private final Map> shuttledExpressionsToExpressionsMap; // Record the exprId and the corresponding expr map, this is used by expression shuttled private final Map namedExprIdAndExprMapping; + private final List planOutputShuttledExpressions; /** * The construct method for StructInfo @@ -125,30 +126,25 @@ private StructInfo(Plan originalPlan, ObjectId originalPlanId, HyperGraph hyperG @Nullable Predicates predicates, Map> shuttledExpressionsToExpressionsMap, Map namedExprIdAndExprMapping, - BitSet tableIdSet) { + BitSet tableIdSet, + SplitPredicate splitPredicate, + EquivalenceClass equivalenceClass, + List planOutputShuttledExpressions) { this.originalPlan = originalPlan; this.originalPlanId = originalPlanId; this.hyperGraph = hyperGraph; - this.valid = valid - && hyperGraph.getNodes().stream().allMatch(n -> ((StructInfoNode) n).getExpressions() != null); + this.valid = valid; this.topPlan = topPlan; this.bottomPlan = bottomPlan; this.relations = relations; this.tableBitSet = tableIdSet; this.relationIdStructInfoNodeMap = relationIdStructInfoNodeMap; this.predicates = predicates; - if (predicates == null) { - // collect predicate from top plan which not in hyper graph - Set topPlanPredicates = new LinkedHashSet<>(); - topPlan.accept(PREDICATE_COLLECTOR, topPlanPredicates); - this.predicates = Predicates.of(topPlanPredicates); - } - Pair derivedPredicates = - predicatesDerive(this.predicates, topPlan, tableBitSet); - this.splitPredicate = derivedPredicates.key(); - this.equivalenceClass = derivedPredicates.value(); + this.splitPredicate = splitPredicate; + this.equivalenceClass = equivalenceClass; this.shuttledExpressionsToExpressionsMap = shuttledExpressionsToExpressionsMap; this.namedExprIdAndExprMapping = namedExprIdAndExprMapping; + this.planOutputShuttledExpressions = planOutputShuttledExpressions; } /** @@ -157,7 +153,8 @@ private StructInfo(Plan originalPlan, ObjectId originalPlanId, HyperGraph hyperG public StructInfo withPredicates(Predicates predicates) { return new StructInfo(this.originalPlan, this.originalPlanId, this.hyperGraph, this.valid, this.topPlan, this.bottomPlan, this.relations, this.relationIdStructInfoNodeMap, predicates, - this.shuttledExpressionsToExpressionsMap, this.namedExprIdAndExprMapping, this.tableBitSet); + this.shuttledExpressionsToExpressionsMap, this.namedExprIdAndExprMapping, this.tableBitSet, + null, null, this.planOutputShuttledExpressions); } /** @@ -166,7 +163,8 @@ public StructInfo withPredicates(Predicates predicates) { public StructInfo withTableBitSet(BitSet tableBitSet) { return new StructInfo(this.originalPlan, this.originalPlanId, this.hyperGraph, this.valid, this.topPlan, this.bottomPlan, this.relations, this.relationIdStructInfoNodeMap, this.predicates, - this.shuttledExpressionsToExpressionsMap, this.namedExprIdAndExprMapping, tableBitSet); + this.shuttledExpressionsToExpressionsMap, this.namedExprIdAndExprMapping, tableBitSet, + this.splitPredicate, this.equivalenceClass, this.planOutputShuttledExpressions); } private static boolean collectStructInfoFromGraph(HyperGraph hyperGraph, @@ -252,11 +250,10 @@ private static boolean collectStructInfoFromGraph(HyperGraph hyperGraph, } // derive some useful predicate by predicates - private Pair predicatesDerive(Predicates predicates, Plan originalPlan, - BitSet tableBitSet) { + private static Pair predicatesDerive(Predicates predicates, Plan originalPlan) { // construct equivalenceClass according to equals predicates List shuttledExpression = ExpressionUtils.shuttleExpressionWithLineage( - new ArrayList<>(predicates.getPulledUpPredicates()), originalPlan, tableBitSet).stream() + new ArrayList<>(predicates.getPulledUpPredicates()), originalPlan, new BitSet()).stream() .map(Expression.class::cast) .collect(Collectors.toList()); SplitPredicate splitPredicate = Predicates.splitPredicates(ExpressionUtils.and(shuttledExpression)); @@ -328,9 +325,19 @@ public static StructInfo of(Plan originalPlan, @Nullable Plan topPlan, @Nullable relationIdStructInfoNodeMap, tableBitSet, cascadesContext); + valid = valid + && hyperGraph.getNodes().stream().allMatch(n -> ((StructInfoNode) n).getExpressions() != null); + // collect predicate from top plan which not in hyper graph + Set topPlanPredicates = new LinkedHashSet<>(); + topPlan.accept(PREDICATE_COLLECTOR, topPlanPredicates); + Predicates predicates = Predicates.of(topPlanPredicates); + // this should use the output of originalPlan to make sure the output right order + List planOutputShuttledExpressions = + ExpressionUtils.shuttleExpressionWithLineage(originalPlan.getOutput(), originalPlan, new BitSet()); return new StructInfo(originalPlan, originalPlanId, hyperGraph, valid, topPlan, bottomPlan, - relationList, relationIdStructInfoNodeMap, null, shuttledHashConjunctsToConjunctsMap, - namedExprIdAndExprMapping, tableBitSet); + relationList, relationIdStructInfoNodeMap, predicates, shuttledHashConjunctsToConjunctsMap, + namedExprIdAndExprMapping, tableBitSet, null, null, + planOutputShuttledExpressions); } /** @@ -350,10 +357,6 @@ public Predicates getPredicates() { return predicates; } - public EquivalenceClass getEquivalenceClass() { - return equivalenceClass; - } - public Plan getOriginalPlan() { return originalPlan; } @@ -362,8 +365,28 @@ public HyperGraph getHyperGraph() { return hyperGraph; } + /** + * lazy init for performance + */ public SplitPredicate getSplitPredicate() { - return splitPredicate; + if (this.splitPredicate == null && this.predicates != null) { + Pair derivedPredicates = predicatesDerive(this.predicates, topPlan); + this.splitPredicate = derivedPredicates.key(); + this.equivalenceClass = derivedPredicates.value(); + } + return this.splitPredicate; + } + + /** + * lazy init for performance + */ + public EquivalenceClass getEquivalenceClass() { + if (this.equivalenceClass == null && this.predicates != null) { + Pair derivedPredicates = predicatesDerive(this.predicates, topPlan); + this.splitPredicate = derivedPredicates.key(); + this.equivalenceClass = derivedPredicates.value(); + } + return this.equivalenceClass; } public boolean isValid() { @@ -416,6 +439,10 @@ public BitSet getTableBitSet() { return tableBitSet; } + public List getPlanOutputShuttledExpressions() { + return planOutputShuttledExpressions; + } + /** * Judge the source graph logical is whether the same as target * For inner join should judge only the join tables, diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/mv/HyperGraphAggTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/mv/HyperGraphAggTest.java index 40b759c8a6bb19..44939cc61c6784 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/mv/HyperGraphAggTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/mv/HyperGraphAggTest.java @@ -89,9 +89,9 @@ void testIJWithAgg() { } LogicalCompatibilityContext constructContext(Plan p1, Plan p2) { - StructInfo st1 = MaterializedViewUtils.extractStructInfo(p1, + StructInfo st1 = MaterializedViewUtils.extractStructInfo(p1, p1, null, new BitSet()).get(0); - StructInfo st2 = MaterializedViewUtils.extractStructInfo(p2, + StructInfo st2 = MaterializedViewUtils.extractStructInfo(p2, p2, null, new BitSet()).get(0); RelationMapping rm = RelationMapping.generate(st1.getRelations(), st2.getRelations()).get(0); SlotMapping sm = SlotMapping.generate(rm); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/sqltest/SqlTestBase.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/sqltest/SqlTestBase.java index 887ae3e29212dd..78918b2ee1b907 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/sqltest/SqlTestBase.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/sqltest/SqlTestBase.java @@ -100,9 +100,9 @@ protected void runBeforeEach() throws Exception { } protected LogicalCompatibilityContext constructContext(Plan p1, Plan p2, CascadesContext context) { - StructInfo st1 = MaterializedViewUtils.extractStructInfo(p1, + StructInfo st1 = MaterializedViewUtils.extractStructInfo(p1, p1, context, new BitSet()).get(0); - StructInfo st2 = MaterializedViewUtils.extractStructInfo(p2, + StructInfo st2 = MaterializedViewUtils.extractStructInfo(p2, p2, context, new BitSet()).get(0); RelationMapping rm = RelationMapping.generate(st1.getRelations(), st2.getRelations()).get(0); SlotMapping sm = SlotMapping.generate(rm);