924060929 commented on code in PR #45045:
URL: https://github.com/apache/doris/pull/45045#discussion_r1886266697


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/TableCollector.java:
##########
@@ -0,0 +1,73 @@
+// 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.
+
+package org.apache.doris.nereids.jobs.executor;
+
+import org.apache.doris.nereids.CascadesContext;
+import org.apache.doris.nereids.jobs.rewrite.RewriteJob;
+import org.apache.doris.nereids.rules.analysis.CollectRelation;
+import org.apache.doris.nereids.rules.analysis.CollectTableFromCTE;
+
+import com.google.common.collect.ImmutableSet;
+
+import java.util.List;
+
+/**
+ * Bind symbols according to metadata in the catalog, perform semantic 
analysis, etc.
+ * TODO: revisit the interface after subquery analysis is supported.
+ */
+public class TableCollector extends AbstractBatchJobExecutor {
+
+    public static final List<RewriteJob> COLLECT_JOBS = 
buildCollectTableJobs();
+
+    /**
+     * constructor of Analyzer. For view, we only do bind relation since other 
analyze step will do by outer Analyzer.
+     *
+     * @param cascadesContext current context for analyzer
+     */
+    public TableCollector(CascadesContext cascadesContext) {
+        super(cascadesContext);
+
+    }
+
+    @Override
+    public List<RewriteJob> getJobs() {
+        return COLLECT_JOBS;
+    }
+
+    /**
+     * nereids analyze sql.
+     */
+    public void collect() {
+        execute();
+    }
+
+    private static List<RewriteJob> buildCollectTableJobs() {
+        return notTraverseChildrenOf(
+                ImmutableSet.of(),
+                TableCollector::buildCollectorJobs
+        );
+    }
+
+    private static List<RewriteJob> buildCollectorJobs() {
+        return jobs(
+                // should collect table from cte first to fill collect all cte 
name to avoid collect wrong table.
+                topDown(new CollectTableFromCTE()),
+                topDown(new CollectRelation())

Review Comment:
   merge this rules in one top down iteration?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectRelation.java:
##########
@@ -0,0 +1,190 @@
+// 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.
+
+package org.apache.doris.nereids.rules.analysis;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.MTMV;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.View;
+import org.apache.doris.mtmv.BaseTableInfo;
+import org.apache.doris.mtmv.MTMVUtil;
+import org.apache.doris.nereids.CTEContext;
+import org.apache.doris.nereids.CascadesContext;
+import org.apache.doris.nereids.StatementContext.TableFrom;
+import org.apache.doris.nereids.analyzer.UnboundRelation;
+import org.apache.doris.nereids.analyzer.UnboundResultSink;
+import org.apache.doris.nereids.analyzer.UnboundTableSink;
+import org.apache.doris.nereids.parser.NereidsParser;
+import org.apache.doris.nereids.pattern.MatchingContext;
+import org.apache.doris.nereids.properties.PhysicalProperties;
+import org.apache.doris.nereids.rules.Rule;
+import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.SubqueryExpr;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
+import org.apache.doris.nereids.util.RelationUtil;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Rule to bind relations in query plan.
+ */
+public class CollectRelation implements AnalysisRuleFactory {
+
+    public CollectRelation() {}
+
+    @Override
+    public List<Rule> buildRules() {
+        return ImmutableList.of(
+                unboundRelation()
+                        .thenApply(this::collectFromUnboundRelation)
+                        .toRule(RuleType.COLLECT_TABLE_FROM_RELATION),
+                unboundTableSink()
+                        .thenApply(this::collectFromUnboundTableSink)
+                        .toRule(RuleType.COLLECT_TABLE_FROM_SINK),
+                any().whenNot(UnboundRelation.class::isInstance)
+                        .whenNot(UnboundTableSink.class::isInstance)
+                        .thenApply(this::collectFromAny)
+                        .toRule(RuleType.COLLECT_TABLE_FROM_OTHER)
+        );
+    }
+
+    private Plan collectFromAny(MatchingContext<Plan> ctx) {
+        Set<SubqueryExpr> subqueryExprs = ctx.root.getExpressions().stream()
+                .<Set<SubqueryExpr>>map(p -> 
p.collect(SubqueryExpr.class::isInstance))
+                .flatMap(Set::stream)
+                .collect(Collectors.toSet());
+        for (SubqueryExpr subqueryExpr : subqueryExprs) {
+            CascadesContext subqueryContext = 
CascadesContext.newContextWithCteContext(
+                    ctx.cascadesContext, subqueryExpr.getQueryPlan(), 
ctx.cteContext);
+            
subqueryContext.keepOrShowPlanProcess(ctx.cascadesContext.showPlanProcess(),
+                    () -> subqueryContext.newTableCollector().collect());
+            
ctx.cascadesContext.addPlanProcesses(subqueryContext.getPlanProcesses());
+        }
+        return null;
+    }
+
+    private Plan 
collectFromUnboundTableSink(MatchingContext<UnboundTableSink<Plan>> ctx) {
+        List<String> nameParts = ctx.root.getNameParts();
+        switch (nameParts.size()) {
+            case 1:
+                // table
+                // Use current database name from catalog.
+            case 2:
+                // db.table
+                // Use database name from table name parts.
+            case 3:
+                // catalog.db.table
+                // Use catalog and database name from name parts.
+                collectFromUnboundRelation(ctx.cascadesContext, nameParts, 
TableFrom.INSERT_TARGET, false);
+                return null;
+            default:
+                throw new IllegalStateException("Insert target name is 
invalid.");
+        }
+    }
+
+    private Plan collectFromUnboundRelation(MatchingContext<UnboundRelation> 
ctx) {
+        List<String> nameParts = ctx.root.getNameParts();
+        switch (nameParts.size()) {
+            case 1:
+                // table
+                // Use current database name from catalog.
+            case 2:
+                // db.table
+                // Use database name from table name parts.
+            case 3:
+                // catalog.db.table
+                // Use catalog and database name from name parts.
+                collectFromUnboundRelation(ctx.cascadesContext, nameParts, 
TableFrom.QUERY, true);
+                return null;
+            default:
+                throw new IllegalStateException("Table name [" + 
ctx.root.getTableName() + "] is invalid.");
+        }
+    }
+
+    private void collectFromUnboundRelation(CascadesContext cascadesContext,
+            List<String> nameParts, TableFrom tableFrom, boolean collectMTMV) {
+        if (nameParts.size() == 1) {
+            String tableName = nameParts.get(0);
+            // check if it is a CTE's name
+            CTEContext cteContext = 
cascadesContext.getCteContext().findCTEContext(tableName).orElse(null);
+            if (cteContext != null) {
+                Optional<LogicalPlan> analyzedCte = 
cteContext.getAnalyzedCTEPlan(tableName);
+                if (analyzedCte.isPresent()) {
+                    return;
+                }
+            }
+        }
+        List<String> tableQualifier = 
RelationUtil.getQualifierName(cascadesContext.getConnectContext(), nameParts);
+        TableIf table = 
cascadesContext.getConnectContext().getStatementContext()
+                .getAndCacheTable(tableQualifier, tableFrom);
+        if (collectMTMV) {
+            collectMTMVCandidates(table, cascadesContext);
+        }
+        if (table instanceof View) {
+            processView((View) table, cascadesContext);
+        }
+    }
+
+    private void collectMTMVCandidates(TableIf table, CascadesContext 
cascadesContext) {
+        if 
(cascadesContext.getConnectContext().getSessionVariable().enableMaterializedViewRewrite)
 {
+            Set<MTMV> mtmvSet = 
Env.getCurrentEnv().getMtmvService().getRelationManager()
+                    .getAvailableMTMVs(Lists.newArrayList(new 
BaseTableInfo(table)),
+                            cascadesContext.getConnectContext(),
+                            false, ((connectContext, mtmv)
+                                    -> MTMVUtil.mtmvContainsExternalTable(mtmv)
+                                    && !connectContext.getSessionVariable()
+                                    
.isEnableMaterializedViewRewriteWhenBaseTableUnawareness()));
+            for (MTMV mtmv : mtmvSet) {
+                
cascadesContext.getStatementContext().getMtmvRelatedTables().put(mtmv.getFullQualifiers(),
 mtmv);
+                mtmv.readMvLock();
+                try {
+                    for (BaseTableInfo baseTableInfo : 
mtmv.getRelation().getBaseTables()) {
+                        
cascadesContext.getStatementContext().getAndCacheTable(baseTableInfo.toList(), 
TableFrom.MTMV);
+                    }
+                } finally {
+                    mtmv.readMvUnlock();
+                }
+            }
+        }
+    }
+
+    private void processView(View view, CascadesContext cascadesContext) {
+        String inlineViewDef = view.getInlineViewDef();
+        parseAndCollectFromView(inlineViewDef, cascadesContext);
+    }
+
+    private void parseAndCollectFromView(String ddlSql, CascadesContext 
parentContext) {
+        LogicalPlan parsedViewPlan = new NereidsParser().parseSingle(ddlSql);
+        // TODO: use a good to do this, such as eliminate UnboundResultSink
+        if (parsedViewPlan instanceof UnboundResultSink) {
+            parsedViewPlan = (LogicalPlan) ((UnboundResultSink<?>) 
parsedViewPlan).child();
+        }
+        CascadesContext viewContext = CascadesContext.initContext(
+                parentContext.getStatementContext(), parsedViewPlan, 
PhysicalProperties.ANY);
+        viewContext.keepOrShowPlanProcess(parentContext.showPlanProcess(),
+                () -> viewContext.newTableCollector().collect());
+        parentContext.addPlanProcesses(viewContext.getPlanProcesses());
+    }

Review Comment:
   parsedViewPlan  not replace to the outer logical plan after parse and lead 
to parse view again?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java:
##########
@@ -381,40 +352,31 @@ default void replayDropConstraint(String name) {
     }
 
     default void dropConstraint(String name, boolean replay) {
-        writeLock();
-        try {
-            Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
-            if (!constraintMap.containsKey(name)) {
-                throw new AnalysisException(
-                        String.format("Unknown constraint %s on table %s.", 
name, this.getName()));
-            }
-            Constraint constraint = constraintMap.get(name);
-            constraintMap.remove(name);
-            if (constraint instanceof PrimaryKeyConstraint) {
-                ((PrimaryKeyConstraint) constraint).getForeignTables()
-                        .forEach(t -> t.dropFKReferringPK(this, 
(PrimaryKeyConstraint) constraint));
-            }
-            if (!replay) {
-                Env.getCurrentEnv().getEditLog().logDropConstraint(new 
AlterConstraintLog(constraint, this));
-            }
-        } finally {
-            writeUnlock();
+        Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
+        if (!constraintMap.containsKey(name)) {
+            throw new AnalysisException(
+                    String.format("Unknown constraint %s on table %s.", name, 
this.getName()));
+        }
+        Constraint constraint = constraintMap.get(name);
+        constraintMap.remove(name);
+        if (constraint instanceof PrimaryKeyConstraint) {
+            ((PrimaryKeyConstraint) constraint).getForeignTables()
+                    .forEach(t -> t.dropFKReferringPK(this, 
(PrimaryKeyConstraint) constraint));
+        }
+        if (!replay) {
+            Env.getCurrentEnv().getEditLog().logDropConstraint(new 
AlterConstraintLog(constraint, this));
         }
     }
 
     default void dropFKReferringPK(TableIf table, PrimaryKeyConstraint 
constraint) {
-        writeLock();
-        try {
-            Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
-            Set<String> fkName = constraintMap.entrySet().stream()
-                    .filter(e -> e.getValue() instanceof ForeignKeyConstraint
-                    && ((ForeignKeyConstraint) 
e.getValue()).isReferringPK(table, constraint))
-                    .map(Entry::getKey)
-                    .collect(Collectors.toSet());
-            fkName.forEach(constraintMap::remove);
-        } finally {
-            writeUnlock();
-        }
+        Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
+        Set<String> fkName = constraintMap.entrySet().stream()
+                .filter(e -> e.getValue() instanceof ForeignKeyConstraint
+                        && ((ForeignKeyConstraint) 
e.getValue()).isReferringPK(table, constraint))
+                .map(Entry::getKey)
+                .collect(Collectors.toSet());
+        fkName.forEach(constraintMap::remove);

Review Comment:
   no lock will see the state of partial update?



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to