morrySnow commented on code in PR #67891:
URL: https://github.com/apache/doris/pull/67891#discussion_r4079838197
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -142,21 +143,27 @@ public boolean isForeignKey(Set<Slot> key) {
}
public boolean isPrimaryKey(Set<Slot> key) {
- return primaryKeys.containsAll(
- key.stream().map(s ->
slotToColumn.get(s)).collect(Collectors.toSet()));
+ return !key.isEmpty() && activePrimaryKeySlots.containsAll(key);
}
void putSlot(SlotReference slot, TableIf table) {
if (!slot.getOriginalColumn().isPresent()) {
return;
}
Column c = slot.getOriginalColumn().get();
- slotToColumn.put(slot, new QualifiedColumn(table, c));
+ QualifiedColumn qualifiedColumn = new QualifiedColumn(table, c);
+ slotToColumn.put(slot, qualifiedColumn);
+ if (declaredPrimaryKeys.contains(qualifiedColumn)) {
+ activePrimaryKeySlots.add(slot);
Review Comment:
Fixed in 6eacb9874ce. canUseCurrentConstraint now rejects
LogicalOlapScan.isDuplicateProducingScanMode() as well as scanParams, so
skip_delete_bitmap, skip_storage_engine_merge, and table-scoped
read_mor_as_dup_tables cannot contribute PK or FK proof lineage. Added the
fk_raw_version_foreign regression with a raw UNIQUE MOR foreign scan: the raw
scan exposes both historical rows, INNER_JOIN is retained, and only the
currently matching row is returned.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -119,47 +163,185 @@ 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);
}
+ /**
+ * 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()) {
+ /**
+ * 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;
+ }
+ 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 this table's
+ * complete declared primary keys if the scan covers the full relation.
Passing only local
+ * declarations avoids revisiting keys from every previously visited
table; scan eligibility
+ * is computed once regardless of how many keys this table declares.
+ *
+ * @param relation catalog scan contributing the slots and relation
identity
+ * @param table catalog table containing the declared columns
+ * @param tablePrimaryKeys declared PK column sets belonging to this
scan's table
+ */
+ void putSlots(LogicalCatalogRelation relation, TableIf table,
+ Set<Set<QualifiedColumn>> tablePrimaryKeys) {
+ 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);
+ }
+
+ if (tablePrimaryKeys.isEmpty() || !canActivatePrimaryKey(relation)) {
Review Comment:
Follow-up fixed in 6eacb9874ce. The current-state guard now applies
duplicate-producing raw-version scan semantics before any foreign-key slots are
registered, instead of checking them only during primary-key activation. The
new actual-result regression demonstrates that a historical foreign version
would otherwise leak through join elimination.
--
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]
