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


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorTable.java:
##########
@@ -61,33 +60,16 @@ public TupleDescriptor getTupleDesc(TupleId id) {
         return tupleDescs.get(id);
     }
 
-    public TDescriptorTable toThrift() {
-        if (thriftDescTable != null) {
-            return thriftDescTable;
-        }
+    public TDescriptorTable getThriftDescTable() {
+        return thriftDescTable;
+    }
 
-        TDescriptorTable result = new TDescriptorTable();
-        Map<Long, TableIf> referencedTbls = Maps.newHashMap();
-        for (TupleDescriptor tupleD : tupleDescs.values()) {
-            // inline view of a non-constant select has a non-materialized 
tuple descriptor
-            // in the descriptor table just for type checking, which we need 
to skip
-            result.addToTupleDescriptors(tupleD.toThrift());
-            // an inline view of a constant select has a materialized tuple
-            // but its table has no id
-            if (tupleD.getTable() != null
-                    && tupleD.getTable().getId() >= 0) {
-                referencedTbls.put(tupleD.getTable().getId(), 
tupleD.getTable());
-            }
-            for (SlotDescriptor slotD : tupleD.getSlots()) {
-                result.addToSlotDescriptors(slotD.toThrift());
-            }
-        }
+    void setThriftDescTable(TDescriptorTable thriftDescTable) {
+        this.thriftDescTable = thriftDescTable;
+    }
 
-        for (TableIf tbl : referencedTbls.values()) {
-            result.addToTableDescriptors(tbl.toThrift());
-        }
-        thriftDescTable = result;
-        return result;

Review Comment:
   `getTupleDescs()` exposes a live `tupleDescs.values()` view as public API. 
That collection supports `remove()`/`clear()`, so external callers can now 
mutate `DescriptorTable` without updating `slotDescs` or the ID generators, 
leaving the object internally inconsistent. Since `DescriptorToThriftConverter` 
is in the same package, this helper can stay package-private, or it should at 
least return an unmodifiable view.



-- 
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