morrySnow commented on code in PR #67891:
URL: https://github.com/apache/doris/pull/67891#discussion_r4081276658
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -119,47 +164,218 @@ 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);
+ }
+
+ /**
+ * 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);
}
- void putSlot(SlotReference slot, TableIf table) {
- if (!slot.getOriginalColumn().isPresent()) {
+ /**
+ * Register a current-state scan's table columns and relation instance for
both FK and PK
+ * proofs. Historical snapshots, change reads, and raw-version scans
cannot use the current
+ * constraint metadata: even if their slots are not active PKs, recording
their FK lineage
+ * could eliminate a join against a different table version. Activate only
this table's
+ * complete PKs when the scan covers the full relation; local declarations
avoid revisiting
+ * earlier tables' keys.
+ *
+ * @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) {
+ if (!canUseCurrentConstraint(relation)) {
+ return;
+ }
+ 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)) {
return;
}
- Column c = slot.getOriginalColumn().get();
- slotToColumn.put(slot, new QualifiedColumn(table, c));
+ for (Set<QualifiedColumn> primaryKey : tablePrimaryKeys) {
+ if (!columnToSlot.keySet().containsAll(primaryKey)) {
+ continue;
+ }
+ Set<Slot> primaryKeySlots = primaryKey.stream()
+ .map(columnToSlot::get)
+ .collect(ImmutableSet.toImmutableSet());
+ activePrimaryKeySlots.addAll(primaryKeySlots);
+ }
+ }
+
+ /**
+ * Check whether a scan reads the current table state assumed by its
declared constraints.
+ * A subset of current rows can still use an FK proof, but historical
snapshots, explicit
+ * branches/tags/options, native or external change reads, and raw-version
scan modes may have
+ * different relationships from the current PK table. Stream scans are
conservatively excluded
+ * for the same reason. Raw-version modes are rejected here, rather than
only when activating a
+ * PK, because a historical foreign row can also make join elimination
unsound.
+ *
+ * @param relation catalog scan whose version and read mode are inspected
+ * @return true if no known version selector, change-read mode, or
raw-version mode is active
+ */
+ boolean canUseCurrentConstraint(LogicalCatalogRelation relation) {
+ if (relation instanceof LogicalOlapTableStreamScan) {
+ return false;
+ }
+ if (relation instanceof LogicalOlapScan) {
+ LogicalOlapScan scan = (LogicalOlapScan) relation;
+ return !scan.getScanParams().isPresent()
+ && !scan.isDuplicateProducingScanMode();
+ }
+ if (relation instanceof LogicalFileScan) {
+ LogicalFileScan scan = (LogicalFileScan) relation;
+ return !scan.getTableSnapshot().isPresent() &&
!scan.getScanParams().isPresent();
+ }
+ return true;
+ }
+
+ /**
+ * Determine whether a scan reads the full relation described by its
declared primary key.
+ * This checks relation coverage after current-state eligibility has
rejected versioned and
+ * duplicate-producing reads. It deliberately does not check the data
trait's inferred
+ * uniqueness: PK constraints are declarative assumptions, and a trait
check is not a
+ * validation of stored data.
+ *
+ * @param relation scan whose output is compared with the declared table
relation
+ * @return true if no known scan selector or mode invalidates the PK proof
+ */
+ boolean canActivatePrimaryKey(LogicalCatalogRelation relation) {
+ if (!canUseCurrentConstraint(relation)) {
+ return false;
+ }
+ if (relation instanceof LogicalOlapScan) {
+ LogicalOlapScan scan = (LogicalOlapScan) relation;
+ return new HashSet<>(scan.getSelectedPartitionIds()).equals(
+ new HashSet<>(scan.getTable().getPartitionIds()))
+ && scan.getSelectedTabletIds().isEmpty()
+ && !scan.getTableSample().isPresent()
+ && !scan.isDirectMvScan();
Review Comment:
Fixed in 42b8fd27bf7. `isDirectMvScan()` is now rejected in
`canUseCurrentConstraint()` before `putSlots` records any slot lineage, so a
direct rollup can supply neither FK nor PK proof; the former PK-only check was
removed. Added unit coverage for the shared gate and an actual-result
`fk_direct_index_foreign` regression using an AGG_KEYS rollup that drops `g2`
and changes two `parent_id = 1` rows into `parent_id = 2`. The fixed plan
retains `INNER_JOIN` and returns count 0; the old plan eliminated the join and
returned 1. `EliminateJoinByFkTest` passes 27/27, the regression passes, and
the FE build/Checkstyle pass.
--
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]
