github-actions[bot] commented on code in PR #66287:
URL: https://github.com/apache/doris/pull/66287#discussion_r3746321533


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectRelation.java:
##########
@@ -314,9 +314,29 @@ protected void parseAndCollectFromView(List<String> 
tableQualifier, View view, C
     }
 
     private void collectFromTableStream(BaseTableStream tableStream, 
CascadesContext cascadesContext,
-                                        TableFrom tableFrom, 
Optional<UnboundRelation> unboundRelation) {
+                                        TableFrom tableFrom) {
         StatementContext statementContext = 
cascadesContext.getConnectContext().getStatementContext();
-        List<String> tableQualifier = tableStream.getBaseTableFullQualifiers();
-        statementContext.getAndCacheTable(tableQualifier, tableFrom, 
unboundRelation);
+        TableIf baseTable = tableStream.getBaseTableNullable();
+        if (baseTable == null) {
+            throw new AnalysisException("Table ["
+                    + tableStream.getBaseTableFullQualifiers().get(2) + "] 
does not exist");
+        }
+
+        // Cache the ID-resolved base table so planner locks the object used 
during binding.
+        Map<List<String>, TableIf> tables;
+        switch (tableFrom) {
+            case QUERY:
+                tables = statementContext.getTables();
+                break;
+            case INSERT_TARGET:
+                tables = statementContext.getInsertTargetTables();
+                break;
+            case MTMV:
+                tables = statementContext.getMtmvRelatedTables();
+                break;
+            default:
+                throw new AnalysisException("Unknown table from " + tableFrom);
+        }
+        tables.put(baseTable.getFullQualifiers(), baseTable);

Review Comment:
   **[P1] Keep stream lock dependencies out of the relation cache**
   
   Reduced plan:
   
   ```text
   Join
     UnboundRelation(db.b)  // table B
     UnboundRelation(db.s)  // stream base A, currently named a
   ```
   
   If collection first caches `db.b -> B`, another session can rename `B: b -> 
c` and `A: a -> b` before the collector visits `s`. This unconditional `put` 
then replaces the already-resolved `db.b` entry with A. 
`StatementContext.lock()` omits B, and `BindRelation.getAndCacheTable(db.b, 
...)` returns A, so the bound tree scans A for both branches and can silently 
return the wrong rows. This is distinct from the existing replacement-table 
thread: the stream base is resolved by the correct stable ID here, but the new 
write corrupts an explicit relation already fenced in the cache. Please keep 
implicit base lock resources in a separate ID/object-keyed collection consumed 
by `lock()`, and cover this pre-lock rename interleaving with a 
latch-controlled test.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to