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


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -131,35 +182,154 @@ void putAllPrimaryKeys(TableIf table) {
         for (PrimaryKeyConstraint c : 
Env.getCurrentEnv().getConstraintManager()
                 .getPrimaryKeyConstraints(tableNameInfo)) {
             Set<QualifiedColumn> primaryKey = c.getPrimaryKeys(table).stream()
-                    .map(column -> new QualifiedColumn(table, 
column)).collect(Collectors.toSet());
-            primaryKeys.addAll(primaryKey);
+                    .map(column -> new QualifiedColumn(table, column))
+                    .collect(ImmutableSet.toImmutableSet());
+            primaryKeys.add(primaryKey);
         }
     }
 
+    /**
+     * Check that the slots are exactly one declared foreign key from one 
relation instance.
+     * Matching only table-qualified columns would incorrectly combine 
components from two aliases
+     * of the same table; {@code slotToRelationId} prevents that combination.
+     *
+     * @param key candidate foreign-side join slots
+     * @return true only for a complete declared FK from one scan instance
+     */
     public boolean isForeignKey(Set<Slot> key) {
-        return foreignKeys.containsAll(
-                key.stream().map(s -> 
slotToColumn.get(s)).collect(Collectors.toSet()));
+        return matchesDeclaredKey(key, foreignKeyColumnSets);
     }
 
+    /**
+     * Check that all slots still have an active scan proof and form a 
complete declared primary
+     * key of one relation instance. Alias combinations are checked without 
storing every variant.
+     *
+     * @param key candidate primary-side join slots
+     * @return true only while a complete declared PK remains active
+     */
     public boolean isPrimaryKey(Set<Slot> key) {
-        return primaryKeys.containsAll(
-                key.stream().map(s -> 
slotToColumn.get(s)).collect(Collectors.toSet()));
+        return activePrimaryKeySlots.containsAll(key) && 
matchesDeclaredKey(key, primaryKeys);
     }
 
-    void putSlot(SlotReference slot, TableIf table) {
-        if (!slot.getOriginalColumn().isPresent()) {
-            return;
+    /**
+     * Match a slot set against declared keys without collapsing repeated 
columns or mixing
+     * relation instances. The size comparison rejects two aliases of one 
component being treated
+     * as two distinct components of a composite key.
+     *
+     * @param key candidate slots from a join condition
+     * @param declaredKeys table-qualified PK or FK column sets
+     * @return true if the slots exactly match one declared key from one scan 
instance
+     */
+    private boolean matchesDeclaredKey(Set<Slot> key, 
Set<Set<QualifiedColumn>> declaredKeys) {
+        if (key.isEmpty()) {
+            return false;
+        }
+        RelationId relationId = slotToRelationId.get(key.iterator().next());
+        if (relationId == null || key.stream().anyMatch(slot -> 
!relationId.equals(slotToRelationId.get(slot)))) {
+            return false;
         }
-        Column c = slot.getOriginalColumn().get();
-        slotToColumn.put(slot, new QualifiedColumn(table, c));
+        Set<QualifiedColumn> columns = key.stream()
+                .map(slotToColumn::get)
+                .collect(Collectors.toSet());
+        return key.size() == columns.size()
+                && !columns.contains(null)
+                && declaredKeys.contains(columns);
     }
 
+    /**
+     * Register each scan slot's table column and relation instance, then 
activate the slots of
+     * complete declared primary keys when scan selectors still cover the full 
relation.
+     *
+     * @param relation catalog scan contributing the slots and relation 
identity
+     * @param table catalog table containing the declared columns
+     */
+    void putSlots(LogicalCatalogRelation relation, TableIf table) {
+        Map<QualifiedColumn, Slot> columnToSlot = new HashMap<>();
+        for (Slot slot : relation.getOutput()) {
+            if (!(slot instanceof SlotReference) || !((SlotReference) 
slot).getOriginalColumn().isPresent()) {
+                continue;
+            }
+            Column column = ((SlotReference) slot).getOriginalColumn().get();
+            QualifiedColumn qualifiedColumn = new QualifiedColumn(table, 
column);
+            slotToColumn.put(slot, qualifiedColumn);
+            slotToRelationId.put(slot, relation.getRelationId());
+            columnToSlot.put(qualifiedColumn, slot);
+        }
+
+        for (Set<QualifiedColumn> primaryKey : primaryKeys) {

Review Comment:
   [P2] Avoid rescanning every table's keys at each scan
   
   `primaryKeys` is context-wide, so after visiting `j` PK-bearing relations 
this loop checks all `j` declarations even though `columnToSlot` contains only 
the current table. A subtree with `N` distinct constrained scans therefore 
performs `1 + ... + N = O(N^2)` unrelated-key checks; because the bottom-up 
rule builds a fresh context for nested `Project(Join)` candidates, a reachable 
left-deep query compounds this to `O(N^3)`. This is distinct from the earlier 
alias power-set thread: it occurs with one-column keys and no aliases. Please 
index declarations by owning `TableIdentifier` (or activate only the current 
table's declarations), compute the scan gate once, and add a many-relation 
bound.



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