github-actions[bot] commented on code in PR #67891:
URL: https://github.com/apache/doris/pull/67891#discussion_r4069459352
##########
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:
[P1] Preserve nullable FK rows in MV comparison
This proof is also consumed by
`HyperGraphComparator.canEliminatePrimaryByForeign`, which only checks that
`canEliminateByFk` succeeds before removing a view-only primary node. Unlike
`EliminateJoinByFK`, that path adds no `IS NOT NULL` compensation.
```text
Query:
Project(f.parent_id)
Scan f
MV:
Project(f.parent_id)
InnerJoin(p.id = f.parent_id)
Scan p
Scan f
```
With `p={1}` and nullable `f.parent_id={1,NULL}`, the query returns both
rows while the MV has already discarded `NULL`. Removing `p` makes the graphs
appear compatible even though the MV cannot answer the query. Please reject
nullable FK slots in this consumer unless the query proves them non-null, and
add an MV rewrite/result regression.
--
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]
