morrySnow commented on code in PR #67891:
URL: https://github.com/apache/doris/pull/67891#discussion_r4070017371


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -119,47 +164,213 @@ void putAllForeignKeys(TableIf table) {
                             entry -> new QualifiedColumn(
                                     referencedTable, 
referencedTable.getColumn(entry.getValue()))));
             constraints.add(constraint);
-            foreignKeys.addAll(constraint.keySet());
+            foreignKeyColumnSets.add(constraint.keySet());
         }
     }
 
-    void putAllPrimaryKeys(TableIf table) {
+    /**
+     * Load a table's declared primary-key column sets into the context-wide 
lookup, then return
+     * only this table's declarations for scan activation. The declaration is 
trusted as metadata;
+     * whether a particular scan can use it is decided separately by
+     * {@link #canActivatePrimaryKey(LogicalCatalogRelation)}.
+     *
+     * @param table catalog table whose PK declarations should be registered
+     * @return declared primary keys belonging to this table, excluding 
unrelated tables' keys
+     */
+    Set<Set<QualifiedColumn>> putAllPrimaryKeys(TableIf table) {
+        Set<Set<QualifiedColumn>> tablePrimaryKeys = new HashSet<>();
         TableNameInfo tableNameInfo = 
TableNameInfoUtils.fromTableOrNull(table);
         if (tableNameInfo == null) {
-            return;
+            return tablePrimaryKeys;
         }
         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());
+            tablePrimaryKeys.add(primaryKey);
+            primaryKeys.add(primaryKey);
         }
+        return tablePrimaryKeys;
     }
 
+    /**
+     * 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);

Review Comment:
   Fixed in c57fa20bf89. The MV hypergraph comparator now carries each nullable 
view-side FK slot as a separate non-null requirement into the existing 
predicate-compensation path. Without a query-side null-rejection proof, MV 
rewriting is rejected; with `f.parent_id IS NOT NULL`, it remains allowed. 
Before the fix I reproduced the `use_mv` query returning only `(1,1)` and 
losing `(2,NULL)`. The new MV rewrite/result regression verifies both rows are 
preserved for the unfiltered query and the filtered query still uses the MV. FE 
unit tests pass (27 executed, 0 failures), the FE build/Checkstyle pass, and 
the sandbox regression passes.



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