github-actions[bot] commented on code in PR #67891:
URL: https://github.com/apache/doris/pull/67891#discussion_r4082453333
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/HyperGraphComparator.java:
##########
@@ -187,14 +189,32 @@ private ComparisonResult isLogicCompatible() {
return new LogicalProject<>(projects, basePlan);
}
+ /**
+ * Check whether an MV-only primary node can be removed using its
foreign-key join.
+ * The MV's inner join has already discarded rows with any null FK column.
Record each
+ * nullable FK slot separately so predicate compensation requires the
query to discard
+ * those same rows before the comparison can be used for an MV rewrite.
+ *
+ * @param primaryNodes bitmap of the primary-key side of the MV join
+ * @param foreignNodes bitmap of the foreign-key side of the MV join
+ * @param primarySlots primary-side slots needed to reconstruct the join
proof
+ * @param foreignSlots foreign-side slots needed to reconstruct the join
proof
+ * @param joinEdge MV join edge whose primary side may be removed
+ * @return true if the PK-FK proof holds; nullable FK slots still require
query compensation
+ */
private boolean canEliminatePrimaryByForeign(long primaryNodes, long
foreignNodes,
Set<Slot> primarySlots, Set<Slot> foreignSlots, JoinEdge joinEdge)
{
Plan foreign = constructViewPlan(foreignNodes, foreignSlots);
Plan primary = constructViewPlan(primaryNodes, primarySlots);
if (foreign == null || primary == null) {
return false;
}
- return JoinUtils.canEliminateByFk(joinEdge.getJoin(), primary,
foreign) != null;
+ Pair<Set<Slot>, Set<Slot>> proof =
JoinUtils.canEliminateByFk(joinEdge.getJoin(), primary, foreign);
+ if (proof == null) {
+ return false;
+ }
+
proof.second.stream().filter(Slot::nullable).forEach(nullableForeignKeySlotsToReject::add);
Review Comment:
[P1] Keep this null rejection scoped below outer joins
This records only the nullable FK slot, so the compensation path can use an
INNER edge anywhere in the query and materialize that scoped proof as a
top-level `IS NOT NULL` filter over the MV scan. A reachable pair is:
```text
Query: MV:
LeftJoin(q.id = f.k) LeftJoin(q.id = f.k)
Scan q Scan q
InnerJoin(f.parent_id=x.id) InnerJoin(f.parent_id=x.id)
Scan f InnerJoin(f.parent_id=p.id)
Scan x Scan f
Scan p
Scan x
```
With nullable `f.parent_id` referencing `p.id`, eliminating MV-only `p`
records this slot. The common `f-x` INNER join then satisfies
`getInnerJoinNullRejectSlots`, but
`getQueryBasedNullRejectCompensationPredicates` emits `f.parent_id IS NOT NULL`
above the MV scan. With one `q` row and no matching `f`, both source trees
retain `(q.id, NULL)` from the LEFT join, while that top-level filter removes
it. Please retain the rejecting edge/dominance scope and reject the rewrite or
place compensation below intervening null-introducing joins; add a nested
INNER-under-LEFT result regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java:
##########
@@ -119,47 +164,221 @@ 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, raw-version
scan modes, and direct
+ * indexes may have different relationships from the current PK table. In
particular, a rollup
+ * can merge base rows and change an FK column's value while retaining its
catalog column
+ * identity. Stream scans are conservatively excluded for the same reason.
These modes are
+ * rejected here, rather than only when activating a PK, because a changed
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,
raw-version mode, or direct
+ * index can change the constraint-bearing rows
+ */
+ boolean canUseCurrentConstraint(LogicalCatalogRelation relation) {
+ if (relation instanceof LogicalOlapTableStreamScan) {
+ return false;
+ }
+ if (relation instanceof LogicalOlapScan) {
+ LogicalOlapScan scan = (LogicalOlapScan) relation;
+ return !scan.getScanParams().isPresent()
+ && !scan.isDuplicateProducingScanMode()
+ && !scan.isDirectMvScan();
+ }
+ 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(
Review Comment:
[P1] Reject duplicate partition selectors before activating the PK proof
Converting both sides to sets makes `p PARTITION(p,p)` look like a full scan
of an unpartitioned table. The parser, binder, and `LogicalOlapScan` preserve
both occurrences, and physical translation passes that list to `OlapScanNode`;
`computeTabletInfo` iterates both IDs and appends the same tablet scan ranges
twice. A query such as `SELECT f.* FROM f JOIN p PARTITION(p,p) ON f.pid =
p.id` therefore has two primary-side matches per foreign row without
elimination, but only one row after this gate activates the PK proof and
removes `p`. Please also require one-to-one cardinality with the table
partition list (or deduplicate/reject repeated selectors before proof
activation), and add an actual-result regression for the repeated table-name
partition case.
--
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]
