github-actions[bot] commented on code in PR #61176:
URL: https://github.com/apache/doris/pull/61176#discussion_r2910210772
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java:
##########
@@ -20,6 +20,7 @@
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.Pair;
import org.apache.doris.common.util.MetaLockUtils;
+import org.apache.doris.mtmv.MTMVUtil;
import org.apache.doris.nereids.NereidsPlanner;
Review Comment:
**Issue [Medium]: Missing cache invalidation for the referenced table in FK
constraints.**
For foreign key constraints, `addForeignConstraint()` also mutates the
*referenced* (primary) table's `PrimaryKeyConstraint` via
`primaryKeyConstraint.addForeignTable(this)`. MTMVs that depend on the
referenced table may use FK metadata for rewrite decisions (e.g., PK-FK join
elimination). Their caches should also be invalidated.
Suggested fix:
```java
MTMVUtil.clearDependentMtmvCaches(columnsAndTable.second);
if (referencedColumnsAndTable != null) {
MTMVUtil.clearDependentMtmvCaches(referencedColumnsAndTable.second);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java:
##########
@@ -21,6 +21,7 @@
import org.apache.doris.catalog.constraint.Constraint;
import org.apache.doris.catalog.constraint.ForeignKeyConstraint;
import org.apache.doris.catalog.constraint.PrimaryKeyConstraint;
+import org.apache.doris.mtmv.MTMVUtil;
import org.apache.doris.nereids.NereidsPlanner;
Review Comment:
**Issue [Medium]: Missing cache invalidation for the referenced table when
dropping FK constraints.**
When dropping a FK constraint, `dropConstraintRef()` also modifies the
referenced (primary) table via `primaryKeyConstraint.removeForeignTable(...)`.
Similarly, when dropping a PK constraint, the FK tables' constraints may be
affected. MTMVs depending on those related tables should also have their caches
cleared.
Note: `getConstraintRelatedTables()` (lines 62-70) already exists and
identifies these related tables but is never called. It should be used here.
The constraint type must be captured *before* the drop (since `dropConstraint`
removes it from the map).
Suggested approach:
```java
// Before dropping, under the read lock or at start of write lock block:
Constraint constraint = table.getConstraintsMapUnsafe().get(name);
List<TableIf> relatedTables = getConstraintRelatedTables(constraint);
// After dropping and unlocking:
MTMVUtil.clearDependentMtmvCaches(table);
for (TableIf related : relatedTables) {
MTMVUtil.clearDependentMtmvCaches(related);
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVUtil.java:
##########
@@ -98,6 +98,24 @@ public static MTMV getMTMV(long dbId, long mtmvId) throws
DdlException, MetaNotF
return (MTMV) db.getTableOrMetaException(mtmvId,
TableType.MATERIALIZED_VIEW);
}
+ /**
+ * Clear cached plans for all MTMVs that depend on the given base table.
+ */
+ public static void clearDependentMtmvCaches(TableIf table) throws
DdlException {
+ BaseTableInfo tableInfo = new BaseTableInfo(table);
+ try {
+ for (BaseTableInfo mtmvInfo
+ : Env.getCurrentEnv().getMtmvService().getRelationManager()
+ .getMtmvsByBaseTable(tableInfo)) {
+ MTMV mtmv = MTMVUtil.getMTMV(mtmvInfo);
+ mtmv.clearCache();
+ }
+ } catch (Exception e) {
+ throw new DdlException(String.format(
Review Comment:
**Issue [Low]: Consider making cache clearing best-effort rather than
failing the DDL.**
This method is called *after* the constraint has already been persisted via
EditLog. If `clearDependentMtmvCaches` throws (e.g., MTMV was concurrently
dropped, metadata lookup fails), the user sees a `DdlException` even though the
constraint change is already committed and will be replayed on followers. This
is confusing — the DDL succeeded but appears to have failed.
Consider logging the error and continuing instead:
```java
} catch (Exception e) {
LOG.warn("failed to clear dependent mtmv caches for base table {}",
tableInfo, e);
}
```
The stale cache will be rebuilt on the next MTMV refresh or FE restart
anyway, so a transient failure here is harmless.
--
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]
