fix regression comment

apache · Apr 21, 2024 · c9a6d24 · c9a6d24
1 parent b736ea3
commit c9a6d24
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 27 deletions.
diff --git a/...main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java b/...main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java
@@ -141,20 +141,22 @@ public List<Plan> rewrite(Plan queryPlan, CascadesContext cascadesContext) {
      */
     protected List<StructInfo> getValidQueryStructInfos(Plan queryPlan, CascadesContext cascadesContext,
             BitSet materializedViewTableSet) {
-        return MaterializedViewUtils.extractStructInfo(queryPlan, cascadesContext, materializedViewTableSet)
-                .stream()
-                .filter(queryStructInfo -> {
-                    boolean valid = checkPattern(queryStructInfo);
-                    if (!valid) {
-                        cascadesContext.getMaterializationContexts().forEach(ctx ->
-                                ctx.recordFailReason(queryStructInfo, "Query struct info is invalid",
-                                        () -> String.format("query table bitmap is %s, plan is %s",
-                                                queryStructInfo.getTableBitSet(), queryPlan.treeString())
-                                ));
-                    }
-                    return valid;
-                })
-                .collect(Collectors.toList());
+        List<StructInfo> validStructInfos = new ArrayList<>();
+        List<StructInfo> uncheckedStructInfos = MaterializedViewUtils.extractStructInfo(queryPlan, cascadesContext,
+                materializedViewTableSet);
+        uncheckedStructInfos.forEach(queryStructInfo -> {
+            boolean valid = checkPattern(queryStructInfo);
+            if (!valid) {
+                cascadesContext.getMaterializationContexts().forEach(ctx ->
+                        ctx.recordFailReason(queryStructInfo, "Query struct info is invalid",
+                                () -> String.format("query table bitmap is %s, plan is %s",
+                                        queryStructInfo.getTableBitSet(), queryPlan.treeString())
+                        ));
+            } else {
+                validStructInfos.add(queryStructInfo);
+            }
+        });
+        return validStructInfos;
     }
 
     /**

diff --git a/...re/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java b/...re/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java
@@ -50,7 +50,6 @@
 
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 
@@ -151,13 +150,19 @@ public static List<StructInfo> extractStructInfo(Plan plan, CascadesContext casc
             StructInfoMap structInfoMap = ownerGroup.getstructInfoMap();
             structInfoMap.refresh(ownerGroup);
             Set<BitSet> queryTableSets = structInfoMap.getTableMaps();
+            ImmutableList.Builder<StructInfo> structInfosBuilder = ImmutableList.builder();
             if (!queryTableSets.isEmpty()) {
-                return queryTableSets.stream()
-                        // Just construct the struct info which mv table set contains all the query table set
-                        .filter(queryTableSet -> materializedViewTableSet.isEmpty()
-                                || StructInfo.containsAll(materializedViewTableSet, queryTableSet))
-                        .map(tableMap -> structInfoMap.getStructInfo(tableMap, tableMap, ownerGroup, plan))
-                        .collect(Collectors.toList());
+                for (BitSet queryTableSet : queryTableSets) {
+                    if (!materializedViewTableSet.isEmpty()
+                            && !StructInfo.containsAll(materializedViewTableSet, queryTableSet)) {
+                        continue;
+                    }
+                    StructInfo structInfo = structInfoMap.getStructInfo(queryTableSet, queryTableSet, ownerGroup, plan);
+                    if (structInfo != null) {
+                        structInfosBuilder.add(structInfo);
+                    }
+                }
+                return structInfosBuilder.build();
             }
         }
         // if plan doesn't belong to any group, construct it directly
@@ -176,8 +181,8 @@ public static Plan generateMvScanPlan(MTMV materializedView, CascadesContext cas
                 materializedView,
                 ImmutableList.of(materializedView.getQualifiedDbName()),
                 // this must be empty, or it will be used to sample
-                Lists.newArrayList(),
-                Lists.newArrayList(),
+                ImmutableList.of(),
+                ImmutableList.of(),
                 Optional.empty());
         mvScan = mvScan.withMaterializedIndexSelected(PreAggStatus.on(), materializedView.getBaseIndexId());
         List<NamedExpression> mvProjects = mvScan.getOutput().stream().map(NamedExpression.class::cast)

diff --git a/...src/main/java/org/apache/doris/nereids/trees/plans/visitor/ExpressionLineageReplacer.java b/...src/main/java/org/apache/doris/nereids/trees/plans/visitor/ExpressionLineageReplacer.java
@@ -45,12 +45,12 @@
  * Get from rewrite plan and can also get from plan struct info, if from plan struct info it depends on
  * the nodes from graph.
  */
-public class ExpressionLineageReplacer extends DefaultPlanVisitor<Void, ExpressionReplaceContext> {
+public class ExpressionLineageReplacer extends DefaultPlanVisitor<Expression, ExpressionReplaceContext> {
 
     public static final ExpressionLineageReplacer INSTANCE = new ExpressionLineageReplacer();
 
     @Override
-    public Void visit(Plan plan, ExpressionReplaceContext context) {
+    public Expression visit(Plan plan, ExpressionReplaceContext context) {
         List<? extends Expression> expressions = plan.getExpressions();
         Map<ExprId, Expression> targetExpressionMap = context.getExprIdExpressionMap();
         // Filter the namedExpression used by target and collect the namedExpression
@@ -62,7 +62,7 @@ public Void visit(Plan plan, ExpressionReplaceContext context) {
     }
 
     @Override
-    public Void visitGroupPlan(GroupPlan groupPlan, ExpressionReplaceContext context) {
+    public Expression visitGroupPlan(GroupPlan groupPlan, ExpressionReplaceContext context) {
         Group group = groupPlan.getGroup();
         if (group == null) {
             return visit(groupPlan, context);

diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
@@ -1646,7 +1646,7 @@ public void setEnableLeftZigZag(boolean enableLeftZigZag) {
                     "When the materialized view is not enough to provide all the data for the query, "
                             + "whether to allow the union of the base table and the materialized view to "
                             + "respond to the query"})
-    public boolean enableMaterializedViewUnionRewrite = true;
+    public boolean enableMaterializedViewUnionRewrite = false;
 
     @VariableMgr.VarAttr(name = CREATE_TABLE_PARTITION_MAX_NUM, needForward = true,
             description = {"建表时创建分区的最大数量",

diff --git a/regression-test/suites/nereids_rules_p0/mv/union_rewrite/usercase_union_rewrite.groovy b/regression-test/suites/nereids_rules_p0/mv/union_rewrite/usercase_union_rewrite.groovy
@@ -164,6 +164,7 @@ suite ("usercase_union_rewrite") {
     compare_res(query_stmt + " order by 1,2,3,4,5,6,7,8")
 
     sql """insert into orders_user values (5, 5, 'k', 99.5, 'a', 'b', 1, 'yy', '2023-10-19');"""
+    sql "SET enable_materialized_view_union_rewrite=true"
     sleep(10 * 1000)
     explain {
         sql("${query_stmt}")