From 72a6f95a658c7dae7c741345396fe158bdda41a9 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Sat, 7 Mar 2026 00:40:21 +0800 Subject: [PATCH 1/2] [Enhancement](constraints) Refactor constraint management into centralized ConstraintManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create ConstraintManager class with ConcurrentHashMap storage keyed by fully qualified table name (catalog.db.table) - Move all constraint CRUD operations from TableIf to ConstraintManager - Add own persistence module (image + editlog replay) for constraints - Support cleanup hooks: table drop, database drop, catalog drop, rename - Maintain FK-PK bidirectional references via foreignTableNames/referencedTableName - Backward compatibility: migrate old table-based constraints via GsonPostProcessable and migrateConstraintsFromTables() - Remove all constraint methods from TableIf interface (233 lines) - Update optimizer (ForeignKeyContext), commands, and catalog relations - Use TableNameInfo in AlterConstraintLog for name-based editlog persistence with backward compat migration from old TableIdentifier format - Clear old table constraints after migration to prevent duplicate migration - Deprecate getTableAttributes() on Table/ExternalTable and getConstraintsMap() on TableAttributes - Fix review findings: null-guard in EditLog replay, idempotent replay, volatile referencedTableName, cross-catalog FK cleanup, migration FK ref rebuild, deprecated getForeignTables() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> refactor: Replace String qualifiedTableName with TableNameInfo in constraint APIs - ForeignKeyConstraint: Add referencedTableInfo (TableNameInfo) field alongside referencedTableNameStr for backward compat. Constructor now takes TableNameInfo instead of String. getReferencedTableName() returns TableNameInfo. - PrimaryKeyConstraint: Add foreignTableInfos (List) alongside foreignTableNameStrs for backward compat. addForeignTable/removeForeignTable/ getForeignTableInfos/renameForeignTable now use TableNameInfo. - ConstraintManager: All public methods take TableNameInfo instead of String. Added toKey() helper to convert TableNameInfo to map key string internally. - Updated all callers: EditLog, AddConstraintCommand, DropConstraintCommand, ShowConstraintsCommand, InternalCatalog, Env, LogicalCatalogRelation, PhysicalCatalogRelation, ForeignKeyContext. - Updated tests: ConstraintPersistTest, ConstraintTest. - gsonPostProcess() handles migration from old serialized formats. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Add thread safety and DDL constraint checks - ConstraintManager.addConstraint() now validates table/column existence atomically under write lock to prevent TOCTOU race conditions - Drop table: rejects if PK is referenced by FK (unless FORCE) - Schema change: rejects DROP COLUMN if column is in any constraint - Replace table: handles constraint swap/rename/drop properly - Add 4 new test cases covering all safety scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Allow DropConstraintCommand when table no longer exists External tables can be deleted by other systems outside Doris. When this happens, DropConstraintCommand would fail because it tries to resolve the table via the planner. This change adds a fallback path: if table resolution fails, extract the table name from the UnboundRelation's name parts and fill in catalog/db from the ConnectContext. Also handles 1-part, 2-part, and 3-part table name specifications correctly in the fallback path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Handle tables without database in constraint lookups TableNameInfo(TableIf) throws AnalysisException when the table has no database (e.g., standalone OlapTable objects in unit tests created via PlanConstructor). This broke many rewrite/analysis tests that use such tables in LogicalCatalogRelation.computeUnique/computeFdItems, PhysicalCatalogRelation.computeUnique, and ForeignKeyContext methods. Added TableNameInfo.createOrNull(TableIf) factory method that returns null instead of throwing when the table lacks a database or catalog. All four call sites now use this method and skip constraint lookups when the table has no database context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Address code review findings - Fix TableNameInfo hashCode()/equals() contract violation: hashCode() now uses toString().hashCode() to be consistent with equals() which compares toString() results. Previously hashCode() used Objects.hash(tbl, db, ctl) which included the raw ctl field, while equals()/toString() skipped the 'internal' catalog name. - Add LOG.warn in DropConstraintCommand fallback path to aid debugging when table resolution fails and name-based lookup is used instead. - Fix ShowConstraintsCommand Javadoc: was 'add constraint command', corrected to 'show constraints command'. - Document TOCTOU vs deadlock tradeoff in ConstraintManager.addConstraint Javadoc: validation is kept inside write lock for correctness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Integrate renameTable into all rename paths ConstraintManager.renameTable() existed but was never called from any rename code path, leaving constraints keyed under the old table name after rename (becoming unreachable). Added renameTable() calls to: - Env.renameTable() — internal catalog master path - Env.replayRenameTable() — internal catalog replay path - ExternalCatalog.renameTable() — external catalog master path - RefreshManager.replayRefreshTable() — external catalog replay path Added renameTableUpdatesConstraintsTest to verify constraints are correctly migrated when a table is renamed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> fix(constraints): Eliminate TOCTOU gap in drop table constraint check checkNoReferencingForeignKeys (readLock) and dropTableConstraints (writeLock) were called separately, creating a TOCTOU window where a new FK could be added between the check and the drop. Added checkAndDropTableConstraints() which holds the write lock for both the FK reference check and the constraint drop, making the operation atomic. Updated InternalCatalog.unprotectDropTable to use the new method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Add comprehensive ConstraintManager unit tests Add ~50 direct API tests for ConstraintManager covering: - Basic CRUD (add/get/drop constraints) - Type-specific getters (PK/FK/UNIQUE) - FK bidirectional reference management - Cascade drop (PK drops referencing FKs) - checkAndDropTableConstraints (atomic check + drop) - findConstraintWithColumn - dropCatalogConstraints - renameTable (moves constraints + updates FK refs) - swapTableConstraints - dropAndRenameConstraints - migrateFromTable - Serialization round-trip (write/read) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Fix ConstraintManagerTest review findings - Fix dropCatalogConstraintsCascadesFKsAcrossCatalogs: assert FK on T1 IS cascade-dropped when referenced PK's catalog is dropped (was incorrectly documented as not cascade-dropped) - Add before-assertion in rebuildForeignKeyReferencesWiresFKToPK to verify PK doesn't know about FK table before rebuild - Add swapTableConstraintsUpdatesFKReferences: verify FK cross-references are updated when tables are swapped - Add dropAndRenameUpdatesFKReferences: verify FK cross-references are updated when table is replaced without swap Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../java/org/apache/doris/alter/Alter.java | 13 + .../doris/alter/SchemaChangeHandler.java | 20 + .../java/org/apache/doris/catalog/Env.java | 87 +- .../apache/doris/catalog/RefreshManager.java | 4 + .../java/org/apache/doris/catalog/Table.java | 16 +- .../apache/doris/catalog/TableAttributes.java | 5 + .../org/apache/doris/catalog/TableIf.java | 232 ----- .../catalog/constraint/ConstraintManager.java | 953 ++++++++++++++++++ .../constraint/ForeignKeyConstraint.java | 120 ++- .../constraint/PrimaryKeyConstraint.java | 74 +- .../catalog/constraint/TableIdentifier.java | 13 + .../apache/doris/datasource/CatalogMgr.java | 1 + .../doris/datasource/ExternalCatalog.java | 4 + .../doris/datasource/ExternalTable.java | 12 +- .../doris/datasource/InternalCatalog.java | 13 +- .../org/apache/doris/info/TableNameInfo.java | 34 +- .../rules/rewrite/ForeignKeyContext.java | 24 +- .../plans/commands/AddConstraintCommand.java | 50 +- .../plans/commands/DropConstraintCommand.java | 48 +- .../commands/ShowConstraintsCommand.java | 25 +- .../plans/logical/LogicalCatalogRelation.java | 20 +- .../physical/PhysicalCatalogRelation.java | 11 +- .../doris/persist/AlterConstraintLog.java | 45 +- .../org/apache/doris/persist/EditLog.java | 28 +- .../doris/persist/meta/MetaPersistMethod.java | 6 + .../persist/meta/PersistMetaModules.java | 2 +- .../constraint/ConstraintManagerTest.java | 587 +++++++++++ .../constraint/ConstraintPersistTest.java | 138 ++- .../nereids/trees/plans/ConstraintTest.java | 260 ++++- .../pkfk/test_pk_fk_drop_table.groovy | 12 +- 30 files changed, 2444 insertions(+), 413 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java create mode 100644 fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java index 3e07bb20e983f5..8c00dfedddcd2d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java @@ -828,6 +828,19 @@ private void replaceTableInternal(Database db, OlapTable origTable, OlapTable ne throws DdlException { String oldTblName = origTable.getName(); String newTblName = newTbl.getName(); + + // Handle constraints for table replacement + TableNameInfo origTableInfo = new TableNameInfo(origTable); + TableNameInfo newTableInfo = new TableNameInfo(newTbl); + if (swapTable) { + Env.getCurrentEnv().getConstraintManager().swapTableConstraints(origTableInfo, newTableInfo); + } else { + if (!isReplay) { + Env.getCurrentEnv().getConstraintManager().checkNoReferencingForeignKeys(origTableInfo); + } + Env.getCurrentEnv().getConstraintManager().dropAndRenameConstraints(origTableInfo, newTableInfo); + } + // drop origin table and new table db.unregisterTable(oldTblName); db.unregisterTable(newTblName); diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java index 3aee05b5a8e94d..13f602bf54f7c1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java @@ -86,6 +86,7 @@ import org.apache.doris.common.util.TimeUtils; import org.apache.doris.common.util.Util; import org.apache.doris.datasource.InternalCatalog; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.commands.AlterCommand; import org.apache.doris.nereids.trees.plans.commands.CancelAlterTableCommand; @@ -267,6 +268,15 @@ private void processDropColumn(DropColumnClause alterClause, Table externalTable throws DdlException { String dropColName = alterClause.getColName(); + String constraintName = Env.getCurrentEnv().getConstraintManager() + .findConstraintWithColumn(new TableNameInfo(externalTable), dropColName); + if (constraintName != null) { + throw new DdlException(String.format( + "Cannot drop column '%s' because it is used by constraint '%s'. " + + "Drop the constraint first.", + dropColName, constraintName)); + } + // find column in base index and remove it boolean found = false; Iterator baseIter = newSchema.iterator(); @@ -303,6 +313,16 @@ private boolean processDropColumn(DropColumnClause alterClause, OlapTable olapTa throws DdlException { String dropColName = alterClause.getColName(); + + String constraintName = Env.getCurrentEnv().getConstraintManager() + .findConstraintWithColumn(new TableNameInfo(olapTable), dropColName); + if (constraintName != null) { + throw new DdlException(String.format( + "Cannot drop column '%s' because it is used by constraint '%s'. " + + "Drop the constraint first.", + dropColName, constraintName)); + } + String targetIndexName = alterClause.getRollupName(); checkIndexExists(olapTable, targetIndexName); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java index 265fe780bd74e0..193ac89a748586 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java @@ -52,6 +52,8 @@ import org.apache.doris.catalog.OlapTable.OlapTableState; import org.apache.doris.catalog.Replica.ReplicaStatus; import org.apache.doris.catalog.TableIf.TableType; +import org.apache.doris.catalog.constraint.Constraint; +import org.apache.doris.catalog.constraint.ConstraintManager; import org.apache.doris.clone.ColocateTableCheckerAndBalancer; import org.apache.doris.clone.DynamicPartitionScheduler; import org.apache.doris.clone.TabletChecker; @@ -102,6 +104,7 @@ import org.apache.doris.datasource.ExternalCatalog; import org.apache.doris.datasource.ExternalMetaCacheMgr; import org.apache.doris.datasource.ExternalMetaIdMgr; +import org.apache.doris.datasource.ExternalTable; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.datasource.SplitSourceManager; import org.apache.doris.datasource.es.EsExternalCatalog; @@ -551,6 +554,8 @@ public class Env { private BinlogManager binlogManager; + private ConstraintManager constraintManager; + private BinlogGcer binlogGcer; private QueryCancelWorker queryCancelWorker; @@ -705,6 +710,10 @@ public BinlogManager getBinlogManager() { return binlogManager; } + public ConstraintManager getConstraintManager() { + return constraintManager; + } + public KeyManagerInterface getKeyManager() throws Exception { if (keyManager == null) { throw new Exception("The keyManager is null, possibly due to a missing implementation of KeyManager"); @@ -850,6 +859,7 @@ public Env(boolean isCheckpointCatalog) { this.hiveTransactionMgr = new HiveTransactionMgr(); this.plsqlManager = new PlsqlManager(); this.binlogManager = new BinlogManager(); + this.constraintManager = new ConstraintManager(); this.binlogGcer = new BinlogGcer(); this.columnIdFlusher = new ColumnIdFlushDaemon(); this.queryCancelWorker = new QueryCancelWorker(systemInfo); @@ -1205,6 +1215,7 @@ public void initialize(String[] args) throws Exception { // 3. Load image first and replay edits this.editLog = new EditLog(nodeName); loadImage(this.imageDir); // load image file + migrateConstraintsFromTables(); // migrate old table-based constraints editLog.open(); // open bdb env this.globalTransactionMgr.setEditLog(editLog); this.idGenerator.setEditLog(editLog); @@ -2980,6 +2991,68 @@ public long saveKeyManagerStore(CountingDataOutputStream out, long checksum) thr return checksum; } + public long saveConstraintManager(CountingDataOutputStream out, long checksum) throws IOException { + constraintManager.write(out); + LOG.info("finished save ConstraintManager to image"); + return checksum; + } + + public long loadConstraintManager(DataInputStream in, long checksum) throws IOException { + this.constraintManager = ConstraintManager.read(in); + LOG.info("finished replay ConstraintManager from image"); + return checksum; + } + + /** + * Migrate constraints from old table-based storage to ConstraintManager. + * Called after image loading to handle upgrade from old format. + */ + @SuppressWarnings("unchecked") + public void migrateConstraintsFromTables() { + if (!constraintManager.isEmpty()) { + return; + } + int migratedCount = 0; + for (CatalogIf catalog : catalogMgr.getCopyOfCatalog()) { + for (Object dbObj : catalog.getAllDbs()) { + DatabaseIf db = (DatabaseIf) dbObj; + for (Object tableObj : db.getTables()) { + TableIf table = (TableIf) tableObj; + try { + Map oldConstraints = null; + if (table instanceof Table) { + oldConstraints = ((Table) table) + .getTableAttributes().getConstraintsMap(); + } else if (table instanceof ExternalTable) { + oldConstraints = ((ExternalTable) table) + .getTableAttributes().getConstraintsMap(); + } else { + LOG.debug("Skipping constraint migration for " + + "unsupported table type: {} ({})", + table.getName(), + table.getClass().getSimpleName()); + } + if (oldConstraints != null && !oldConstraints.isEmpty()) { + String qualifiedName = table.getNameWithFullQualifiers(); + constraintManager.migrateFromTable( + new TableNameInfo(qualifiedName), oldConstraints); + migratedCount += oldConstraints.size(); + oldConstraints.clear(); + } + } catch (Exception e) { + LOG.warn("Failed to migrate constraints for table {}", + table.getName(), e); + } + } + } + } + if (migratedCount > 0) { + LOG.info("Migrated {} constraints from old table-based storage " + + "to ConstraintManager", migratedCount); + constraintManager.rebuildForeignKeyReferences(); + } + } + public void createLabelCleaner() { labelCleaner = new MasterDaemon("LoadLabelCleaner", Config.label_clean_interval_second * 1000L) { @Override @@ -4990,12 +5063,12 @@ public void dropView(String catalogName, String dbName, String tableName, boolea } public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, boolean isReplay, - Long recycleTime) { + Long recycleTime) throws DdlException { return getInternalCatalog().unprotectDropTable(db, table, isForceDrop, isReplay, recycleTime); } public void replayDropTable(Database db, long tableId, boolean isForceDrop, - Long recycleTime) throws MetaNotFoundException { + Long recycleTime) throws MetaNotFoundException, DdlException { getInternalCatalog().replayDropTable(db, tableId, isForceDrop, recycleTime); } @@ -5664,6 +5737,11 @@ public void renameTable(Database db, Table table, String newTableName) throws Dd TableInfo tableInfo = TableInfo.createForTableRename(db.getId(), table.getId(), oldTableName, newTableName); editLog.logTableRename(tableInfo); + constraintManager.renameTable( + new TableNameInfo(InternalCatalog.INTERNAL_CATALOG_NAME, + db.getFullName(), oldTableName), + new TableNameInfo(InternalCatalog.INTERNAL_CATALOG_NAME, + db.getFullName(), newTableName)); LOG.info("rename table[{}] to {}", oldTableName, newTableName); } finally { table.writeUnlock(); @@ -5695,6 +5773,11 @@ public void replayRenameTable(TableInfo tableInfo) throws MetaNotFoundException db.unregisterTable(tableName); table.setName(newTableName); db.registerTable(table); + constraintManager.renameTable( + new TableNameInfo(InternalCatalog.INTERNAL_CATALOG_NAME, + db.getFullName(), tableName), + new TableNameInfo(InternalCatalog.INTERNAL_CATALOG_NAME, + db.getFullName(), newTableName)); LOG.info("replay rename table[{}] to {}", tableName, newTableName); } finally { table.writeUnlock(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java index 86b664eaf36078..d5a349503ebdbf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java @@ -30,6 +30,7 @@ import org.apache.doris.datasource.hive.HMSExternalTable; import org.apache.doris.datasource.hive.HiveExternalMetaCache; import org.apache.doris.datasource.iceberg.IcebergExternalTable; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.persist.OperationType; import com.google.common.base.Strings; @@ -185,6 +186,9 @@ public void replayRefreshTable(ExternalObjectLog log) { // this is a rename table op db.get().unregisterTable(log.getTableName()); db.get().resetMetaCacheNames(); + Env.getCurrentEnv().getConstraintManager().renameTable( + new TableNameInfo(catalog.getName(), log.getDbName(), log.getTableName()), + new TableNameInfo(catalog.getName(), log.getDbName(), log.getNewTableName())); } else { List modifiedPartNames = log.getPartitionNames(); List newPartNames = log.getNewPartitionNames(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Table.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Table.java index 9859e28dc12a87..4e59eb43cf6a63 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Table.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Table.java @@ -18,7 +18,6 @@ package org.apache.doris.catalog; import org.apache.doris.alter.AlterCancelException; -import org.apache.doris.catalog.constraint.Constraint; import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; import org.apache.doris.common.ErrorCode; @@ -121,6 +120,7 @@ public abstract class Table extends MetaObject implements Writable, TableIf, Gso @SerializedName(value = "comment") protected String comment = ""; + @Deprecated @SerializedName(value = "ta") protected TableAttributes tableAttributes = new TableAttributes(); @@ -385,13 +385,13 @@ public String getDisplayName() { return isTemporary ? Util.getTempTableDisplayName(name) : name; } - public Constraint getConstraint(String name) { - return getConstraintsMap().get(name); - } - - @Override - public Map getConstraintsMapUnsafe() { - return tableAttributes.getConstraintsMap(); + /** + * @deprecated Use ConstraintManager for constraint access. + * This method will be removed in a future version. + */ + @Deprecated + public TableAttributes getTableAttributes() { + return tableAttributes; } public TableType getType() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableAttributes.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableAttributes.java index 847cdf7265bef1..bc973ce818b9f5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableAttributes.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableAttributes.java @@ -42,6 +42,11 @@ public TableAttributes() { this.visibleVersionTime = System.currentTimeMillis(); } + /** + * @deprecated Constraints are now managed by ConstraintManager. + * This method will be removed in a future version. + */ + @Deprecated public Map getConstraintsMap() { return constraintsMap; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java index ef9a823fd01e31..03a6d62b97aed3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java @@ -18,30 +18,21 @@ package org.apache.doris.catalog; import org.apache.doris.alter.AlterCancelException; -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.catalog.constraint.TableIdentifier; -import org.apache.doris.catalog.constraint.UniqueConstraint; import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.DdlException; import org.apache.doris.common.MetaNotFoundException; import org.apache.doris.common.Pair; -import org.apache.doris.common.util.MetaLockUtils; import org.apache.doris.datasource.systable.SysTable; import org.apache.doris.datasource.systable.TvfSysTable; import org.apache.doris.info.TableValuedFunctionRefInfo; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.functions.table.TableValuedFunction; -import org.apache.doris.persist.AlterConstraintLog; import org.apache.doris.statistics.AnalysisInfo; import org.apache.doris.statistics.BaseAnalysisTask; import org.apache.doris.statistics.ColumnStatistic; import org.apache.doris.thrift.TTableDescriptor; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import org.apache.logging.log4j.LogManager; @@ -50,14 +41,11 @@ import java.io.DataOutput; import java.io.IOException; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; public interface TableIf { Logger LOG = LogManager.getLogger(TableIf.class); @@ -246,226 +234,6 @@ default long getRowCountForNereids() { void write(DataOutput out) throws IOException; - // Don't use it outside due to its thread-unsafe, use get specific constraints instead. - default Map getConstraintsMapUnsafe() { - throw new RuntimeException(String.format("Not implemented constraint for table %s. " - + "And the function can't be called outside, consider get specific function " - + "like getForeignKeyConstraints/getPrimaryKeyConstraints/getUniqueConstraints.", this)); - } - - default Set getForeignKeyConstraints() { - try { - return getConstraintsMapUnsafe().values().stream() - .filter(ForeignKeyConstraint.class::isInstance) - .map(ForeignKeyConstraint.class::cast) - .collect(ImmutableSet.toImmutableSet()); - } catch (Exception ignored) { - return ImmutableSet.of(); - } - } - - default Map getConstraintsMap() { - try { - return ImmutableMap.copyOf(getConstraintsMapUnsafe()); - } catch (Exception ignored) { - return ImmutableMap.of(); - } - } - - default Set getPrimaryKeyConstraints() { - try { - ImmutableSet.Builder constraintBuilder = ImmutableSet.builder(); - for (Constraint constraint : getConstraintsMapUnsafe().values()) { - if (!(constraint instanceof PrimaryKeyConstraint)) { - continue; - } - constraintBuilder.add((PrimaryKeyConstraint) constraint); - } - return constraintBuilder.build(); - } catch (Exception ignored) { - return ImmutableSet.of(); - } - } - - default Set getUniqueConstraints() { - try { - ImmutableSet.Builder constraintBuilder = ImmutableSet.builder(); - for (Constraint constraint : getConstraintsMapUnsafe().values()) { - if (!(constraint instanceof UniqueConstraint)) { - continue; - } - constraintBuilder.add((UniqueConstraint) constraint); - } - return constraintBuilder.build(); - } catch (Exception ignored) { - return ImmutableSet.of(); - } - } - - // Note this function is not thread safe - default void checkConstraintNotExistenceUnsafe(String name, Constraint primaryKeyConstraint, - Map constraintMap) { - if (constraintMap.containsKey(name)) { - throw new RuntimeException(String.format("Constraint name %s has existed", name)); - } - for (Map.Entry entry : constraintMap.entrySet()) { - if (entry.getValue().equals(primaryKeyConstraint)) { - throw new RuntimeException(String.format( - "Constraint %s has existed, named %s", primaryKeyConstraint, entry.getKey())); - } - } - } - - default void addUniqueConstraint(String name, ImmutableList columns, boolean replay) { - Map constraintMap = getConstraintsMapUnsafe(); - UniqueConstraint uniqueConstraint = new UniqueConstraint(name, ImmutableSet.copyOf(columns)); - checkConstraintNotExistenceUnsafe(name, uniqueConstraint, constraintMap); - constraintMap.put(name, uniqueConstraint); - if (!replay) { - Env.getCurrentEnv().getEditLog().logAddConstraint( - new AlterConstraintLog(uniqueConstraint, this)); - } - } - - default void addPrimaryKeyConstraint(String name, ImmutableList columns, boolean replay) { - Map constraintMap = getConstraintsMapUnsafe(); - PrimaryKeyConstraint primaryKeyConstraint = new PrimaryKeyConstraint(name, ImmutableSet.copyOf(columns)); - checkConstraintNotExistenceUnsafe(name, primaryKeyConstraint, constraintMap); - constraintMap.put(name, primaryKeyConstraint); - if (!replay) { - Env.getCurrentEnv().getEditLog().logAddConstraint( - new AlterConstraintLog(primaryKeyConstraint, this)); - } - } - - default PrimaryKeyConstraint tryGetPrimaryKeyForForeignKeyUnsafe( - PrimaryKeyConstraint requirePrimaryKey, TableIf referencedTable) { - Optional primaryKeyConstraint = referencedTable.getConstraintsMapUnsafe().values().stream() - .filter(requirePrimaryKey::equals) - .findFirst(); - if (!primaryKeyConstraint.isPresent()) { - throw new AnalysisException(String.format( - "Foreign key constraint requires a primary key constraint %s in %s", - requirePrimaryKey.getPrimaryKeyNames(), referencedTable.getName())); - } - return ((PrimaryKeyConstraint) (primaryKeyConstraint.get())); - } - - default void addForeignConstraint(String name, ImmutableList columns, - TableIf referencedTable, ImmutableList referencedColumns, boolean replay) { - Map constraintMap = getConstraintsMapUnsafe(); - ForeignKeyConstraint foreignKeyConstraint = new ForeignKeyConstraint(name, columns, referencedTable, - referencedColumns); - checkConstraintNotExistenceUnsafe(name, foreignKeyConstraint, constraintMap); - PrimaryKeyConstraint requirePrimaryKeyName = new PrimaryKeyConstraint(name, - foreignKeyConstraint.getReferencedColumnNames()); - PrimaryKeyConstraint primaryKeyConstraint = tryGetPrimaryKeyForForeignKeyUnsafe(requirePrimaryKeyName, - referencedTable); - primaryKeyConstraint.addForeignTable(this); - constraintMap.put(name, foreignKeyConstraint); - if (!replay) { - Env.getCurrentEnv().getEditLog().logAddConstraint( - new AlterConstraintLog(foreignKeyConstraint, this)); - } - } - - default void replayAddConstraint(Constraint constraint) { - if (constraint instanceof UniqueConstraint) { - UniqueConstraint uniqueConstraint = (UniqueConstraint) constraint; - this.addUniqueConstraint(constraint.getName(), - ImmutableList.copyOf(uniqueConstraint.getUniqueColumnNames()), true); - } else if (constraint instanceof PrimaryKeyConstraint) { - PrimaryKeyConstraint primaryKeyConstraint = (PrimaryKeyConstraint) constraint; - this.addPrimaryKeyConstraint(primaryKeyConstraint.getName(), - ImmutableList.copyOf(primaryKeyConstraint.getPrimaryKeyNames()), true); - } else if (constraint instanceof ForeignKeyConstraint) { - ForeignKeyConstraint foreignKey = (ForeignKeyConstraint) constraint; - this.addForeignConstraint(foreignKey.getName(), - ImmutableList.copyOf(foreignKey.getForeignKeyNames()), - foreignKey.getReferencedTable(), - ImmutableList.copyOf(foreignKey.getReferencedColumnNames()), true); - } - } - - default void replayDropConstraint(String name) { - dropConstraint(name, true); - } - - // when table has foreign key constraint referencing to primary key of other table, - // need to remove this table identifier from primary table's foreign table set when drop this - // when table has primary key constraint, when drop table(this), need to remove the foreign key referenced this - default void removeTableIdentifierFromPrimaryTable() { - Map constraintMap = getConstraintsMapUnsafe(); - for (Constraint constraint : constraintMap.values()) { - dropConstraintRefWithLock(constraint); - } - } - - default void dropConstraintRefWithLock(Constraint constraint) { - List tables = getConstraintRelatedTables(constraint); - tables.sort((Comparator.comparing(TableIf::getId))); - MetaLockUtils.writeLockTables(tables); - try { - dropConstraintRef(constraint); - } finally { - MetaLockUtils.writeUnlockTables(tables); - } - } - - default void dropConstraintRef(Constraint constraint) { - if (constraint instanceof PrimaryKeyConstraint) { - ((PrimaryKeyConstraint) constraint).getForeignTables() - .forEach(t -> t.dropFKReferringPK(this, (PrimaryKeyConstraint) constraint)); - } else if (constraint instanceof ForeignKeyConstraint) { - ForeignKeyConstraint foreignKeyConstraint = (ForeignKeyConstraint) constraint; - Optional primaryTableIf = foreignKeyConstraint.getReferencedTableOrNull(); - if (primaryTableIf.isPresent()) { - Map refTableConstraintMap = primaryTableIf.get().getConstraintsMapUnsafe(); - for (Constraint refTableConstraint : refTableConstraintMap.values()) { - if (refTableConstraint instanceof PrimaryKeyConstraint) { - PrimaryKeyConstraint primaryKeyConstraint = (PrimaryKeyConstraint) refTableConstraint; - primaryKeyConstraint.removeForeignTable(new TableIdentifier(this)); - } - } - } - } - } - - default List getConstraintRelatedTables(Constraint constraint) { - List tables = Lists.newArrayList(); - if (constraint instanceof PrimaryKeyConstraint) { - tables.addAll(((PrimaryKeyConstraint) constraint).getForeignTables()); - } else if (constraint instanceof ForeignKeyConstraint) { - tables.add(((ForeignKeyConstraint) constraint).getReferencedTable()); - } - return tables; - } - - default void dropConstraint(String name, boolean replay) { - Map constraintMap = getConstraintsMapUnsafe(); - if (!constraintMap.containsKey(name)) { - throw new AnalysisException( - String.format("Unknown constraint %s on table %s.", name, this.getName())); - } - Constraint constraint = constraintMap.get(name); - constraintMap.remove(name); - dropConstraintRefWithLock(constraint); - if (!replay) { - Env.getCurrentEnv().getEditLog().logDropConstraint(new AlterConstraintLog(constraint, this)); - } - } - - default void dropFKReferringPK(TableIf table, PrimaryKeyConstraint constraint) { - Map constraintMap = getConstraintsMapUnsafe(); - Set fkName = constraintMap.entrySet().stream() - .filter(e -> e.getValue() instanceof ForeignKeyConstraint - && ((ForeignKeyConstraint) e.getValue()).isReferringPK(table, constraint)) - .map(Entry::getKey) - .collect(Collectors.toSet()); - fkName.forEach(constraintMap::remove); - - } - /** * return true if this kind of table need read lock when doing query plan. * diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java new file mode 100644 index 00000000000000..78745f3eaf61c2 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java @@ -0,0 +1,953 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog.constraint; + +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.TableIf; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.io.Text; +import org.apache.doris.common.io.Writable; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.persist.AlterConstraintLog; +import org.apache.doris.persist.gson.GsonPostProcessable; +import org.apache.doris.persist.gson.GsonUtils; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.gson.annotations.SerializedName; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.io.DataInput; +import java.io.DataOutput; +import java.io.IOException; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; + +/** + * Centralized manager for all table constraints (PK, FK, UNIQUE). + * Constraints are indexed by fully qualified table name (catalog.db.table). + */ +public class ConstraintManager implements Writable, GsonPostProcessable { + + private static final Logger LOG = LogManager.getLogger(ConstraintManager.class); + + @SerializedName("cm") + private final ConcurrentHashMap> constraintsMap + = new ConcurrentHashMap<>(); + + private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + + public ConstraintManager() { + } + + private static String toKey(TableNameInfo tni) { + return tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl(); + } + + /** Returns true if no constraints are stored. */ + public boolean isEmpty() { + return constraintsMap.isEmpty(); + } + + private void readLock() { + lock.readLock().lock(); + } + + private void readUnlock() { + lock.readLock().unlock(); + } + + private void writeLock() { + lock.writeLock().lock(); + } + + private void writeUnlock() { + lock.writeLock().unlock(); + } + + /** + * Add a constraint to the specified table. + * For FK constraints, validates that the referenced PK exists + * and registers bidirectional reference via foreignTableInfos. + * + *

Note: validation is performed inside the write lock to prevent TOCTOU races + * (e.g., table dropped between validation and registration). For external catalogs, + * this could cause longer lock hold times if catalog initialization is slow. + * This tradeoff is intentional — correctness over performance.

+ */ + public void addConstraint(TableNameInfo tableNameInfo, String constraintName, + Constraint constraint, boolean replay) { + String key = toKey(tableNameInfo); + writeLock(); + try { + if (!replay) { + validateTableAndColumns(tableNameInfo, constraint); + } + Map tableConstraints = constraintsMap.computeIfAbsent( + key, k -> new HashMap<>()); + checkConstraintNotExistence(constraintName, constraint, tableConstraints); + if (constraint instanceof ForeignKeyConstraint) { + registerForeignKeyReference( + tableNameInfo, (ForeignKeyConstraint) constraint); + } + tableConstraints.put(constraintName, constraint); + if (!replay) { + logAddConstraint(tableNameInfo, constraint); + } + LOG.info("Added constraint {} on table {}", constraintName, key); + } finally { + writeUnlock(); + } + } + + /** + * Drop a constraint from the specified table. + * For PK constraints, cascade-drops all referencing FKs. + * For FK constraints, updates the referenced PK's foreign table set. + */ + public void dropConstraint(TableNameInfo tableNameInfo, String constraintName, + boolean replay) { + String key = toKey(tableNameInfo); + writeLock(); + try { + Map tableConstraints = constraintsMap.get(key); + if (tableConstraints == null || !tableConstraints.containsKey(constraintName)) { + if (replay) { + LOG.warn("Constraint {} not found on table {} during replay, skipping", + constraintName, key); + return; + } + throw new AnalysisException(String.format( + "Unknown constraint %s on table %s.", + constraintName, key)); + } + Constraint constraint = tableConstraints.remove(constraintName); + cleanupConstraintReferences(tableNameInfo, constraint); + if (tableConstraints.isEmpty()) { + constraintsMap.remove(key); + } + if (!replay) { + logDropConstraint(tableNameInfo, constraint); + } + LOG.info("Dropped constraint {} from table {}", + constraintName, key); + } finally { + writeUnlock(); + } + } + + /** Returns an immutable copy of all constraints for the given table. */ + public Map getConstraints(TableNameInfo tableNameInfo) { + String key = toKey(tableNameInfo); + readLock(); + try { + Map tableConstraints + = constraintsMap.get(key); + if (tableConstraints == null) { + return ImmutableMap.of(); + } + return ImmutableMap.copyOf(tableConstraints); + } finally { + readUnlock(); + } + } + + /** Get a single constraint by name, or null if not found. */ + public Constraint getConstraint(TableNameInfo tableNameInfo, + String constraintName) { + String key = toKey(tableNameInfo); + readLock(); + try { + Map tableConstraints + = constraintsMap.get(key); + if (tableConstraints == null) { + return null; + } + return tableConstraints.get(constraintName); + } finally { + readUnlock(); + } + } + + /** Returns all PrimaryKeyConstraints for the given table. */ + public ImmutableList getPrimaryKeyConstraints( + TableNameInfo tableNameInfo) { + return getConstraintsByType(toKey(tableNameInfo), + PrimaryKeyConstraint.class); + } + + /** Returns all ForeignKeyConstraints for the given table. */ + public ImmutableList getForeignKeyConstraints( + TableNameInfo tableNameInfo) { + return getConstraintsByType(toKey(tableNameInfo), + ForeignKeyConstraint.class); + } + + /** Returns all UniqueConstraints for the given table. */ + public ImmutableList getUniqueConstraints( + TableNameInfo tableNameInfo) { + return getConstraintsByType(toKey(tableNameInfo), + UniqueConstraint.class); + } + + /** + * Remove all constraints for a table and clean up bidirectional references. + * Called when a table is dropped. + */ + /** + * Atomically check for referencing foreign keys and then drop all constraints + * for the given table. Holds the write lock for both operations to prevent + * TOCTOU races where a new FK could be added between the check and the drop. + * + * @param tableNameInfo the table whose constraints are to be dropped + * @param checkForeignKeys if true, throw DdlException if any PK is FK-referenced + */ + public void checkAndDropTableConstraints(TableNameInfo tableNameInfo, + boolean checkForeignKeys) throws DdlException { + String key = toKey(tableNameInfo); + writeLock(); + try { + Map tableConstraints = constraintsMap.get(key); + if (tableConstraints == null) { + return; + } + if (checkForeignKeys) { + for (Constraint c : tableConstraints.values()) { + if (c instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) c; + List fkTables = pk.getForeignTableInfos(); + if (fkTables != null && !fkTables.isEmpty()) { + String fkTableNames = fkTables.stream() + .map(t -> toKey(t)) + .collect(Collectors.joining(", ")); + throw new DdlException(String.format( + "Cannot drop table %s because its primary" + + " key is referenced by foreign key" + + " constraints from table(s): %s." + + " Drop the foreign key constraints" + + " first.", + key, fkTableNames)); + } + } + } + } + constraintsMap.remove(key); + for (Constraint constraint : tableConstraints.values()) { + cleanupConstraintReferences(tableNameInfo, constraint); + } + LOG.info("Dropped all constraints for table {}", key); + } finally { + writeUnlock(); + } + } + + public void dropTableConstraints(TableNameInfo tableNameInfo) { + String key = toKey(tableNameInfo); + writeLock(); + try { + Map tableConstraints + = constraintsMap.remove(key); + if (tableConstraints == null) { + return; + } + for (Constraint constraint : tableConstraints.values()) { + cleanupConstraintReferences(tableNameInfo, constraint); + } + LOG.info("Dropped all constraints for table {}", key); + } finally { + writeUnlock(); + } + } + + /** + * Remove all constraints whose qualified table name starts with + * the given catalog prefix. Called when a catalog is dropped. + */ + public void dropCatalogConstraints(String catalogName) { + writeLock(); + try { + String prefix = catalogName + "."; + List tablesToRemove = constraintsMap.keySet().stream() + .filter(k -> k.startsWith(prefix)) + .collect(Collectors.toList()); + for (String tableName : tablesToRemove) { + Map tableConstraints + = constraintsMap.remove(tableName); + if (tableConstraints != null) { + for (Constraint constraint : tableConstraints.values()) { + cleanupConstraintReferencesOutsideCatalog( + tableName, constraint, prefix); + } + } + } + LOG.info("Dropped all constraints for catalog {}", catalogName); + } finally { + writeUnlock(); + } + } + + /** + * Move constraints from oldTableInfo to newTableInfo + * and update all FK/PK references. Called when a table is renamed. + */ + public void renameTable(TableNameInfo oldTableInfo, + TableNameInfo newTableInfo) { + String oldKey = toKey(oldTableInfo); + String newKey = toKey(newTableInfo); + writeLock(); + try { + // Move this table's own constraints + Map tableConstraints + = constraintsMap.remove(oldKey); + if (tableConstraints != null) { + constraintsMap.put(newKey, tableConstraints); + } + // Update FK/PK references in OTHER tables + for (Map.Entry> entry + : constraintsMap.entrySet()) { + if (entry.getKey().equals(newKey)) { + continue; + } + for (Constraint c : entry.getValue().values()) { + if (c instanceof ForeignKeyConstraint) { + ForeignKeyConstraint fk = (ForeignKeyConstraint) c; + TableNameInfo refInfo = fk.getReferencedTableName(); + if (refInfo != null && oldTableInfo.equals(refInfo)) { + fk.setReferencedTableInfo(newTableInfo); + } + } else if (c instanceof PrimaryKeyConstraint) { + ((PrimaryKeyConstraint) c).renameForeignTable( + oldTableInfo, newTableInfo); + } + } + } + LOG.info("Renamed table constraints from {} to {}", + oldKey, newKey); + } finally { + writeUnlock(); + } + } + + /** + * Migrate constraints from old table-based storage into this manager. + */ + public void migrateFromTable(TableNameInfo tableNameInfo, + Map existingConstraints) { + if (existingConstraints == null || existingConstraints.isEmpty()) { + return; + } + String key = toKey(tableNameInfo); + writeLock(); + try { + Map tableConstraints + = constraintsMap.computeIfAbsent( + key, k -> new HashMap<>()); + tableConstraints.putAll(existingConstraints); + LOG.info("Migrated {} constraints for table {}", + existingConstraints.size(), key); + } finally { + writeUnlock(); + } + } + + /** + * After all tables have been migrated, wire up FK→PK bidirectional + * references that could not be established during per-table migration + * (because the referenced PK table may not have been migrated yet). + */ + public void rebuildForeignKeyReferences() { + writeLock(); + try { + for (Map.Entry> entry + : constraintsMap.entrySet()) { + String fkTableKey = entry.getKey(); + TableNameInfo fkTableInfo = new TableNameInfo(fkTableKey); + for (Constraint c : entry.getValue().values()) { + if (!(c instanceof ForeignKeyConstraint)) { + continue; + } + ForeignKeyConstraint fk = (ForeignKeyConstraint) c; + TableNameInfo refTableInfo = fk.getReferencedTableName(); + if (refTableInfo == null) { + continue; + } + String refTableKey = toKey(refTableInfo); + Map refTableConstraints + = constraintsMap.get(refTableKey); + if (refTableConstraints == null) { + continue; + } + for (Constraint rc : refTableConstraints.values()) { + if (rc instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) rc; + if (pk.getPrimaryKeyNames().equals( + fk.getReferencedColumnNames())) { + pk.addForeignTable(fkTableInfo); + } + } + } + } + } + LOG.info("Rebuilt FK->PK bidirectional references"); + } finally { + writeUnlock(); + } + } + + @Override + public void write(DataOutput out) throws IOException { + String json = GsonUtils.GSON.toJson(this); + Text.writeString(out, json); + } + + /** Deserialize ConstraintManager from DataInput. */ + public static ConstraintManager read(DataInput in) throws IOException { + String json = Text.readString(in); + return GsonUtils.GSON.fromJson(json, ConstraintManager.class); + } + + @Override + public void gsonPostProcess() throws IOException { + LOG.info("ConstraintManager deserialized with {} table entries", + constraintsMap.size()); + } + + // ==================== DDL-support methods ==================== + + /** + * Check if any PK constraint on this table is referenced by FK constraints + * from other tables. Throws DdlException if references exist. + * Used before drop table to prevent orphaned FK references. + */ + public void checkNoReferencingForeignKeys(TableNameInfo tableNameInfo) + throws DdlException { + readLock(); + try { + String key = toKey(tableNameInfo); + Map tableConstraints + = constraintsMap.get(key); + if (tableConstraints == null) { + return; + } + for (Constraint c : tableConstraints.values()) { + if (c instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) c; + List fkTables + = pk.getForeignTableInfos(); + if (fkTables != null && !fkTables.isEmpty()) { + String fkTableNames = fkTables.stream() + .map(t -> toKey(t)) + .collect(Collectors.joining(", ")); + throw new DdlException(String.format( + "Cannot drop table %s because its primary" + + " key is referenced by foreign key" + + " constraints from table(s): %s." + + " Drop the foreign key constraints" + + " first.", + key, fkTableNames)); + } + } + } + } finally { + readUnlock(); + } + } + + /** + * Check if the given column is part of any constraint on the table. + * Returns the constraint name if found, or null if not. + */ + public String findConstraintWithColumn( + TableNameInfo tableNameInfo, String columnName) { + readLock(); + try { + String key = toKey(tableNameInfo); + Map tableConstraints + = constraintsMap.get(key); + if (tableConstraints == null) { + return null; + } + for (Entry entry + : tableConstraints.entrySet()) { + Constraint c = entry.getValue(); + if (c instanceof PrimaryKeyConstraint) { + if (((PrimaryKeyConstraint) c) + .getPrimaryKeyNames() + .contains(columnName)) { + return entry.getKey(); + } + } else if (c instanceof UniqueConstraint) { + if (((UniqueConstraint) c) + .getUniqueColumnNames() + .contains(columnName)) { + return entry.getKey(); + } + } else if (c instanceof ForeignKeyConstraint) { + if (((ForeignKeyConstraint) c) + .getForeignKeyNames() + .contains(columnName)) { + return entry.getKey(); + } + } + } + return null; + } finally { + readUnlock(); + } + } + + /** + * Atomically swap constraint mappings between two tables. + * Used during REPLACE TABLE with SWAP. + * Also updates all FK/PK cross-references. + */ + public void swapTableConstraints(TableNameInfo tableA, + TableNameInfo tableB) { + String keyA = toKey(tableA); + String keyB = toKey(tableB); + writeLock(); + try { + Map constraintsA + = constraintsMap.remove(keyA); + Map constraintsB + = constraintsMap.remove(keyB); + if (constraintsA != null) { + constraintsMap.put(keyB, constraintsA); + } + if (constraintsB != null) { + constraintsMap.put(keyA, constraintsB); + } + // Update FK/PK references in ALL tables + for (Entry> entry + : constraintsMap.entrySet()) { + for (Constraint c : entry.getValue().values()) { + if (c instanceof ForeignKeyConstraint) { + swapForeignKeyReference( + (ForeignKeyConstraint) c, + tableA, tableB); + } else if (c instanceof PrimaryKeyConstraint) { + swapPrimaryKeyForeignTables( + (PrimaryKeyConstraint) c, + tableA, tableB); + } + } + } + LOG.info("Swapped constraints between {} and {}", + keyA, keyB); + } finally { + writeUnlock(); + } + } + + /** + * Drop constraints for oldTable and rename newTable's constraints + * to oldTable's name. Used during REPLACE TABLE without SWAP. + */ + public void dropAndRenameConstraints(TableNameInfo oldTable, + TableNameInfo newTable) { + writeLock(); + try { + // Drop old table constraints (with cleanup) + String oldKey = toKey(oldTable); + Map oldConstraints + = constraintsMap.remove(oldKey); + if (oldConstraints != null) { + for (Constraint c : oldConstraints.values()) { + cleanupConstraintReferences(oldTable, c); + } + } + // Rename new table constraints to old table name + String newKey = toKey(newTable); + Map newConstraints + = constraintsMap.remove(newKey); + if (newConstraints != null) { + constraintsMap.put(oldKey, newConstraints); + } + // Update FK/PK references pointing to newTable → oldTable + for (Entry> entry + : constraintsMap.entrySet()) { + for (Constraint c : entry.getValue().values()) { + if (c instanceof ForeignKeyConstraint) { + ForeignKeyConstraint fk + = (ForeignKeyConstraint) c; + if (newTable.equals( + fk.getReferencedTableName())) { + fk.setReferencedTableInfo(oldTable); + } + } else if (c instanceof PrimaryKeyConstraint) { + ((PrimaryKeyConstraint) c) + .renameForeignTable( + newTable, oldTable); + } + } + } + LOG.info("Dropped constraints for {} and renamed {}" + + " constraints to {}", + oldKey, newKey, oldKey); + } finally { + writeUnlock(); + } + } + + // ==================== Private helpers ==================== + + private void checkConstraintNotExistence(String name, + Constraint constraint, Map constraintMap) { + if (constraintMap.containsKey(name)) { + throw new AnalysisException( + String.format("Constraint name %s has existed", name)); + } + for (Entry entry : constraintMap.entrySet()) { + if (entry.getValue().equals(constraint)) { + throw new AnalysisException(String.format( + "Constraint %s has existed, named %s", + constraint, entry.getKey())); + } + } + } + + /** + * For FK constraints: find the matching PK on the referenced table + * (using FK's referencedTableInfo) and register the FK table in PK's + * foreignTableInfos list. + */ + private void registerForeignKeyReference(TableNameInfo fkTableInfo, + ForeignKeyConstraint fkConstraint) { + TableNameInfo refTableInfo = fkConstraint.getReferencedTableName(); + if (refTableInfo == null) { + throw new AnalysisException( + "Foreign key constraint has no referenced table name"); + } + String refTableKey = toKey(refTableInfo); + Map refTableConstraints + = constraintsMap.get(refTableKey); + if (refTableConstraints == null) { + throw new AnalysisException(String.format( + "Foreign key constraint requires a primary key constraint " + + "%s in %s", + fkConstraint.getReferencedColumnNames(), refTableKey)); + } + boolean found = false; + for (Constraint c : refTableConstraints.values()) { + if (c instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) c; + if (pk.getPrimaryKeyNames().equals( + fkConstraint.getReferencedColumnNames())) { + pk.addForeignTable(fkTableInfo); + found = true; + break; + } + } + } + if (!found) { + throw new AnalysisException(String.format( + "Foreign key constraint requires a primary key constraint " + + "%s in %s", + fkConstraint.getReferencedColumnNames(), refTableKey)); + } + } + + /** + * Clean up bidirectional references when a constraint is removed. + * PK: cascade-drop all FKs in foreign tables that reference this PK. + * FK: remove the FK table from the referenced PK's foreignTableInfos. + */ + private void cleanupConstraintReferences(TableNameInfo tableNameInfo, + Constraint constraint) { + if (constraint instanceof PrimaryKeyConstraint) { + cascadeDropForeignKeys(tableNameInfo, + (PrimaryKeyConstraint) constraint); + } else if (constraint instanceof ForeignKeyConstraint) { + removeForeignKeyFromPK(tableNameInfo, + (ForeignKeyConstraint) constraint); + } + } + + /** + * Similar to cleanupConstraintReferences but only cleans references + * to tables outside the given catalog prefix (used during catalog drop). + */ + private void cleanupConstraintReferencesOutsideCatalog( + String qualifiedTableName, Constraint constraint, + String catalogPrefix) { + if (constraint instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) constraint; + for (TableNameInfo fkTableInfo : pk.getForeignTableInfos()) { + String fkTableKey = toKey(fkTableInfo); + if (fkTableKey.startsWith(catalogPrefix)) { + // intra-catalog; will be removed together + continue; + } + Map fkTableConstraints + = constraintsMap.get(fkTableKey); + if (fkTableConstraints != null) { + TableNameInfo pkTableInfo = new TableNameInfo(qualifiedTableName); + removeFKsReferencingTable(fkTableConstraints, + pkTableInfo, pk); + if (fkTableConstraints.isEmpty()) { + constraintsMap.remove(fkTableKey); + } + } + } + } else if (constraint instanceof ForeignKeyConstraint) { + ForeignKeyConstraint fk = (ForeignKeyConstraint) constraint; + TableNameInfo refTableInfo = fk.getReferencedTableName(); + if (refTableInfo != null) { + String refTableKey = toKey(refTableInfo); + if (!refTableKey.startsWith(catalogPrefix)) { + TableNameInfo fkTableInfo = new TableNameInfo(qualifiedTableName); + removeForeignKeyFromPK(fkTableInfo, fk); + } + } + } + } + + /** + * When a PK is dropped, cascade-drop all FK constraints in the PK's + * registered foreign tables that reference this PK. + */ + private void cascadeDropForeignKeys(TableNameInfo pkTableInfo, + PrimaryKeyConstraint pkConstraint) { + for (TableNameInfo fkTableInfo : pkConstraint.getForeignTableInfos()) { + String fkTableKey = toKey(fkTableInfo); + Map fkTableConstraints + = constraintsMap.get(fkTableKey); + if (fkTableConstraints == null) { + continue; + } + removeFKsReferencingTable(fkTableConstraints, + pkTableInfo, pkConstraint); + if (fkTableConstraints.isEmpty()) { + constraintsMap.remove(fkTableKey); + } + } + } + + private void removeFKsReferencingTable( + Map fkTableConstraints, + TableNameInfo pkTableInfo, PrimaryKeyConstraint pkConstraint) { + Iterator> it + = fkTableConstraints.entrySet().iterator(); + while (it.hasNext()) { + Entry entry = it.next(); + if (entry.getValue() instanceof ForeignKeyConstraint) { + ForeignKeyConstraint fk + = (ForeignKeyConstraint) entry.getValue(); + if (pkTableInfo.equals(fk.getReferencedTableName()) + && fk.getReferencedColumnNames().equals( + pkConstraint.getPrimaryKeyNames())) { + it.remove(); + } + } + } + } + + /** + * When an FK is dropped, remove the FK table from the referenced PK's + * foreignTableInfos list. + */ + private void removeForeignKeyFromPK(TableNameInfo fkTableInfo, + ForeignKeyConstraint fkConstraint) { + TableNameInfo refTableInfo = fkConstraint.getReferencedTableName(); + if (refTableInfo == null) { + return; + } + String refTableKey = toKey(refTableInfo); + Map refTableConstraints + = constraintsMap.get(refTableKey); + if (refTableConstraints == null) { + return; + } + for (Constraint c : refTableConstraints.values()) { + if (c instanceof PrimaryKeyConstraint) { + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) c; + if (pk.getPrimaryKeyNames().equals( + fkConstraint.getReferencedColumnNames())) { + pk.removeForeignTable(fkTableInfo); + break; + } + } + } + } + + @SuppressWarnings("unchecked") + private ImmutableList getConstraintsByType( + String qualifiedTableName, Class type) { + readLock(); + try { + Map tableConstraints + = constraintsMap.get(qualifiedTableName); + if (tableConstraints == null) { + return ImmutableList.of(); + } + ImmutableList.Builder builder = ImmutableList.builder(); + for (Constraint constraint : tableConstraints.values()) { + if (type.isInstance(constraint)) { + builder.add(type.cast(constraint)); + } + } + return builder.build(); + } finally { + readUnlock(); + } + } + + // ==================== Validation helpers ==================== + + /** + * Validate that the table and columns referenced by the constraint + * actually exist. Only called for non-replay operations. + */ + private void validateTableAndColumns(TableNameInfo tableNameInfo, + Constraint constraint) { + TableIf table = resolveTableForValidation(tableNameInfo); + if (constraint instanceof PrimaryKeyConstraint) { + validateColumnsExist(table, + ((PrimaryKeyConstraint) constraint) + .getPrimaryKeyNames(), + toKey(tableNameInfo)); + } else if (constraint instanceof UniqueConstraint) { + validateColumnsExist(table, + ((UniqueConstraint) constraint) + .getUniqueColumnNames(), + toKey(tableNameInfo)); + } else if (constraint instanceof ForeignKeyConstraint) { + ForeignKeyConstraint fk = (ForeignKeyConstraint) constraint; + validateColumnsExist(table, + fk.getForeignKeyNames(), + toKey(tableNameInfo)); + TableNameInfo refTableInfo = fk.getReferencedTableName(); + if (refTableInfo != null) { + TableIf refTable + = resolveTableForValidation(refTableInfo); + validateColumnsExist(refTable, + fk.getReferencedColumnNames(), + toKey(refTableInfo)); + } + } + } + + private TableIf resolveTableForValidation( + TableNameInfo tableNameInfo) { + try { + CatalogIf catalog = Env.getCurrentEnv().getCatalogMgr() + .getCatalog(tableNameInfo.getCtl()); + if (catalog == null) { + throw new AnalysisException( + "Catalog not found: " + + tableNameInfo.getCtl()); + } + DatabaseIf db = catalog.getDbNullable( + tableNameInfo.getDb()); + if (db == null) { + throw new AnalysisException( + "Database not found: " + + tableNameInfo.getDb() + + " in catalog " + + tableNameInfo.getCtl()); + } + TableIf table = db.getTableNullable( + tableNameInfo.getTbl()); + if (table == null) { + throw new AnalysisException( + "Table not found: " + + toKey(tableNameInfo)); + } + return table; + } catch (AnalysisException e) { + throw e; + } catch (Exception e) { + throw new AnalysisException( + "Failed to resolve table " + + toKey(tableNameInfo) + + ": " + e.getMessage()); + } + } + + private void validateColumnsExist(TableIf table, + Collection columnNames, + String qualifiedTableName) { + for (String columnName : columnNames) { + if (table.getColumn(columnName) == null) { + throw new AnalysisException(String.format( + "Column %s does not exist in table %s", + columnName, qualifiedTableName)); + } + } + } + + // ==================== Swap helpers ==================== + + private void swapForeignKeyReference(ForeignKeyConstraint fk, + TableNameInfo tableA, TableNameInfo tableB) { + TableNameInfo ref = fk.getReferencedTableName(); + if (ref == null) { + return; + } + if (tableA.equals(ref)) { + fk.setReferencedTableInfo(tableB); + } else if (tableB.equals(ref)) { + fk.setReferencedTableInfo(tableA); + } + } + + /** + * Swap references to tableA and tableB in a PK's foreign table list. + * Handles correctly the case where only one, both, or neither is + * present. + */ + private void swapPrimaryKeyForeignTables(PrimaryKeyConstraint pk, + TableNameInfo tableA, TableNameInfo tableB) { + List fkInfos = pk.getForeignTableInfos(); + boolean hasA = fkInfos.stream().anyMatch(tableA::equals); + boolean hasB = fkInfos.stream().anyMatch(tableB::equals); + if (hasA && !hasB) { + pk.renameForeignTable(tableA, tableB); + } else if (!hasA && hasB) { + pk.renameForeignTable(tableB, tableA); + } + // If both or neither present, no change needed + } + + // ==================== EditLog integration ==================== + + private void logAddConstraint(TableNameInfo tableNameInfo, + Constraint constraint) { + AlterConstraintLog log = new AlterConstraintLog( + constraint, tableNameInfo); + Env.getCurrentEnv().getEditLog().logAddConstraint(log); + } + + private void logDropConstraint(TableNameInfo tableNameInfo, + Constraint constraint) { + AlterConstraintLog log = new AlterConstraintLog( + constraint, tableNameInfo); + Env.getCurrentEnv().getEditLog().logDropConstraint(log); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ForeignKeyConstraint.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ForeignKeyConstraint.java index db6d9131dc66a6..2e23d345926e30 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ForeignKeyConstraint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ForeignKeyConstraint.java @@ -18,7 +18,10 @@ package org.apache.doris.catalog.constraint; import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.persist.gson.GsonPostProcessable; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; @@ -26,18 +29,26 @@ import com.google.common.collect.ImmutableSet; import com.google.gson.annotations.SerializedName; +import java.io.IOException; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; -public class ForeignKeyConstraint extends Constraint { +public class ForeignKeyConstraint extends Constraint implements GsonPostProcessable { @SerializedName(value = "ftr") private final Map foreignToReference; @SerializedName(value = "rt") private final TableIdentifier referencedTable; + // qualified name string kept for backward-compatible deserialization + @SerializedName(value = "rtn") + private String referencedTableNameStr; + + @SerializedName(value = "rtni") + private volatile TableNameInfo referencedTableInfo; + public ForeignKeyConstraint(String name, List columns, TableIf refTable, List referencedColumns) { super(ConstraintType.FOREIGN_KEY, name); @@ -49,6 +60,29 @@ public ForeignKeyConstraint(String name, List columns, Preconditions.checkArgument(ImmutableSet.copyOf(referencedColumns).size() == referencedColumns.size(), "Reference keys contains duplicate slots."); this.referencedTable = new TableIdentifier(refTable); + this.referencedTableInfo = new TableNameInfo(refTable); + this.referencedTableNameStr = referencedTableInfo.getCtl() + "." + + referencedTableInfo.getDb() + "." + referencedTableInfo.getTbl(); + for (int i = 0; i < columns.size(); i++) { + builder.put(columns.get(i), referencedColumns.get(i)); + } + this.foreignToReference = builder.build(); + } + + public ForeignKeyConstraint(String name, List columns, + TableNameInfo referencedTableInfo, List referencedColumns) { + super(ConstraintType.FOREIGN_KEY, name); + ImmutableMap.Builder builder = new Builder<>(); + Preconditions.checkArgument(columns.size() == referencedColumns.size(), + "Foreign keys' size must be same as the size of reference keys"); + Preconditions.checkArgument(ImmutableSet.copyOf(columns).size() == columns.size(), + "Foreign keys contains duplicate slots."); + Preconditions.checkArgument(ImmutableSet.copyOf(referencedColumns).size() == referencedColumns.size(), + "Reference keys contains duplicate slots."); + this.referencedTable = null; + this.referencedTableInfo = referencedTableInfo; + this.referencedTableNameStr = referencedTableInfo.getCtl() + "." + + referencedTableInfo.getDb() + "." + referencedTableInfo.getTbl(); for (int i = 0; i < columns.size(); i++) { builder.put(columns.get(i), referencedColumns.get(i)); } @@ -77,38 +111,97 @@ public Map getForeignToReference() { public Map getForeignToPrimary(TableIf curTable) { ImmutableMap.Builder columnBuilder = new ImmutableMap.Builder<>(); - TableIf refTable = referencedTable.toTableIf(); + TableIf refTable = resolveReferencedTable(); foreignToReference.forEach((k, v) -> columnBuilder.put(curTable.getColumn(k), refTable.getColumn(v))); return columnBuilder.build(); } public Column getReferencedColumn(String column) { - return getReferencedTable().getColumn(getReferencedColumnName(column)); + return resolveReferencedTable().getColumn(getReferencedColumnName(column)); } public TableIf getReferencedTable() { - return referencedTable.toTableIf(); + return resolveReferencedTable(); } public Optional getReferencedTableOrNull() { - TableIf res = null; try { - res = referencedTable.toTableIf(); + return Optional.of(resolveReferencedTable()); } catch (Exception ignored) { - // do nothing + return Optional.empty(); + } + } + + private TableIf resolveReferencedTable() { + if (referencedTable != null) { + try { + return referencedTable.toTableIf(); + } catch (Exception e) { + // fall through to name-based resolution + } + } + if (referencedTableInfo != null) { + try { + return Env.getCurrentEnv().getCatalogMgr() + .getCatalog(referencedTableInfo.getCtl()) + .getDbOrAnalysisException(referencedTableInfo.getDb()) + .getTableOrAnalysisException(referencedTableInfo.getTbl()); + } catch (Exception e) { + throw new org.apache.doris.nereids.exceptions.AnalysisException( + "Cannot resolve referenced table: " + referencedTableInfo, e); + } + } + Preconditions.checkNotNull(referencedTableNameStr, + "Neither referencedTable nor referencedTableInfo/referencedTableNameStr is set"); + String[] parts = referencedTableNameStr.split("\\.", 3); + Preconditions.checkArgument(parts.length == 3, + "Invalid qualified table name: %s", referencedTableNameStr); + try { + return Env.getCurrentEnv().getCatalogMgr() + .getCatalog(parts[0]).getDbOrAnalysisException(parts[1]) + .getTableOrAnalysisException(parts[2]); + } catch (Exception e) { + throw new org.apache.doris.nereids.exceptions.AnalysisException( + "Cannot resolve referenced table: " + referencedTableNameStr, e); + } + } + + public TableNameInfo getReferencedTableName() { + return referencedTableInfo; + } + + public void setReferencedTableInfo(TableNameInfo info) { + this.referencedTableInfo = info; + this.referencedTableNameStr = info.getCtl() + "." + info.getDb() + "." + info.getTbl(); + } + + @Override + public void gsonPostProcess() throws IOException { + if (referencedTableInfo == null && referencedTableNameStr != null) { + referencedTableInfo = new TableNameInfo(referencedTableNameStr); + } + if (referencedTableInfo == null && referencedTable != null) { + try { + String qualifiedName = referencedTable.toQualifiedName(); + if (qualifiedName != null) { + referencedTableNameStr = qualifiedName; + referencedTableInfo = new TableNameInfo(qualifiedName); + } + } catch (Exception ignored) { + // skip if the referenced table can no longer be resolved + } } - return Optional.ofNullable(res); } - public Boolean isReferringPK(TableIf table, PrimaryKeyConstraint constraint) { + public Boolean isReferringPK(TableNameInfo pkTableInfo, PrimaryKeyConstraint constraint) { return constraint.getPrimaryKeyNames().equals(getPrimaryKeyNames()) - && getReferencedTable().equals(table); + && pkTableInfo.equals(referencedTableInfo); } @Override public int hashCode() { - return Objects.hash(foreignToReference, referencedTable); + return Objects.hash(foreignToReference, referencedTableInfo); } @Override @@ -121,14 +214,15 @@ public boolean equals(Object obj) { } ForeignKeyConstraint other = (ForeignKeyConstraint) obj; return Objects.equals(foreignToReference, other.foreignToReference) - && Objects.equals(referencedTable, other.referencedTable); + && Objects.equals(referencedTableInfo, other.referencedTableInfo); } @Override public String toString() { String foreignKeys = "(" + String.join(", ", foreignToReference.keySet()) + ")"; String primaryKeys = "(" + String.join(", ", foreignToReference.values()) + ")"; - return String.format("FOREIGN KEY %s REFERENCES %s %s", foreignKeys, referencedTable, primaryKeys); + return String.format("FOREIGN KEY %s REFERENCES %s %s", + foreignKeys, referencedTableInfo, primaryKeys); } public String getTypeName() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java index 0f3515a1cd52a6..0876f7b425fe68 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.TableIf; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.persist.gson.GsonPostProcessable; import com.google.common.base.Objects; @@ -29,6 +30,8 @@ import org.apache.logging.log4j.Logger; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -43,6 +46,13 @@ public class PrimaryKeyConstraint extends Constraint implements GsonPostProcessa @SerializedName(value = "ft") private Set foreignTables = new HashSet<>(); + // qualified name strings kept for backward-compatible deserialization + @SerializedName(value = "ftn") + private Set foreignTableNameStrs = new HashSet<>(); + + @SerializedName(value = "ftni") + private List foreignTableInfos = new ArrayList<>(); + public PrimaryKeyConstraint(String name, Set columns) { super(ConstraintType.PRIMARY_KEY, name); this.columns = ImmutableSet.copyOf(columns); @@ -56,10 +66,16 @@ public Set getPrimaryKeys(TableIf table) { return columns.stream().map(table::getColumn).collect(ImmutableSet.toImmutableSet()); } - public void addForeignTable(TableIf table) { - foreignTables.add(new TableIdentifier(table)); + public void addForeignTable(TableNameInfo tni) { + foreignTableInfos.add(tni); + foreignTableNameStrs.add(tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl()); } + /** + * @deprecated Use {@link #getForeignTableInfos()} instead. + * Returns empty for constraints created via the new ConstraintManager. + */ + @Deprecated public List getForeignTables() { ImmutableList.Builder tableIfBuilder = ImmutableList.builder(); for (TableIdentifier tableIdentifier : foreignTables) { @@ -76,11 +92,65 @@ public void removeForeignTable(TableIdentifier tableIdentifier) { foreignTables.remove(tableIdentifier); } + public void removeForeignTable(TableNameInfo tni) { + String key = tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl(); + foreignTableNameStrs.remove(key); + foreignTableInfos.removeIf(info -> + java.util.Objects.equals(info.getCtl(), tni.getCtl()) + && java.util.Objects.equals(info.getDb(), tni.getDb()) + && java.util.Objects.equals(info.getTbl(), tni.getTbl())); + } + + public List getForeignTableInfos() { + return Collections.unmodifiableList(foreignTableInfos); + } + + public void renameForeignTable(TableNameInfo oldInfo, TableNameInfo newInfo) { + String oldKey = oldInfo.getCtl() + "." + oldInfo.getDb() + "." + oldInfo.getTbl(); + if (foreignTableNameStrs.remove(oldKey)) { + String newKey = newInfo.getCtl() + "." + newInfo.getDb() + "." + newInfo.getTbl(); + foreignTableNameStrs.add(newKey); + } + for (int i = 0; i < foreignTableInfos.size(); i++) { + TableNameInfo info = foreignTableInfos.get(i); + if (java.util.Objects.equals(info.getCtl(), oldInfo.getCtl()) + && java.util.Objects.equals(info.getDb(), oldInfo.getDb()) + && java.util.Objects.equals(info.getTbl(), oldInfo.getTbl())) { + foreignTableInfos.set(i, newInfo); + break; + } + } + } + @Override public void gsonPostProcess() throws IOException { if (foreignTables == null) { foreignTables = new HashSet<>(); } + if (foreignTableNameStrs == null) { + foreignTableNameStrs = new HashSet<>(); + } + if (foreignTableInfos == null) { + foreignTableInfos = new ArrayList<>(); + } + if (foreignTableInfos.isEmpty() && !foreignTableNameStrs.isEmpty()) { + for (String qualifiedName : foreignTableNameStrs) { + foreignTableInfos.add(new TableNameInfo(qualifiedName)); + } + } + if (foreignTableInfos.isEmpty() && !foreignTables.isEmpty()) { + for (TableIdentifier tableIdentifier : foreignTables) { + try { + String qualifiedName = tableIdentifier.toQualifiedName(); + if (qualifiedName != null) { + foreignTableNameStrs.add(qualifiedName); + foreignTableInfos.add(new TableNameInfo(qualifiedName)); + } + } catch (Exception ignored) { + // skip entries that can no longer be resolved + } + } + } } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/TableIdentifier.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/TableIdentifier.java index 20b80d18ff3c7f..1807a9452dc953 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/TableIdentifier.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/TableIdentifier.java @@ -80,6 +80,19 @@ public int hashCode() { return Objects.hash(catalogId, databaseId, tableId); } + /** + * Resolve this identifier to a qualified name in the form "catalog.db.table". + * Returns null if the referenced objects no longer exist. + */ + public String toQualifiedName() { + try { + TableIf tableIf = this.toTableIf(); + return tableIf.getNameWithFullQualifiers(); + } catch (Exception e) { + return null; + } + } + @Override public String toString() { TableIf tableIf = this.toTableIf(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java index c83cb64e34847c..ebf3fe213fb771 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java @@ -173,6 +173,7 @@ private void cleanupRemovedCatalog(RemovedCatalog removedCatalog, boolean perman } CatalogIf catalog = removedCatalog.catalog; catalog.onClose(); + Env.getCurrentEnv().getConstraintManager().dropCatalogConstraints(removedCatalog.catalogName); ConnectContext ctx = ConnectContext.get(); if (ctx != null) { ctx.removeLastDBOfCatalog(removedCatalog.catalogName); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java index 2c31414a99fbdf..279b4d5923174e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java @@ -58,6 +58,7 @@ import org.apache.doris.datasource.trinoconnector.TrinoConnectorExternalDatabase; import org.apache.doris.fs.remote.dfs.DFSFileSystem; import org.apache.doris.info.PartitionNamesInfo; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.trees.plans.commands.info.CreateOrReplaceBranchInfo; import org.apache.doris.nereids.trees.plans.commands.info.CreateOrReplaceTagInfo; import org.apache.doris.nereids.trees.plans.commands.info.CreateTableInfo; @@ -1109,6 +1110,9 @@ public void renameTable(String dbName, String oldTableName, String newTableName) } try { metadataOps.renameTable(dbName, oldTableName, newTableName); + Env.getCurrentEnv().getConstraintManager().renameTable( + new TableNameInfo(getName(), dbName, oldTableName), + new TableNameInfo(getName(), dbName, newTableName)); Env.getCurrentEnv().getEditLog() .logRefreshExternalTable( ExternalObjectLog.createForRenameTable(getId(), dbName, oldTableName, newTableName)); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java index d238ab9556dfc7..a03cf823919f3f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java @@ -24,7 +24,6 @@ import org.apache.doris.catalog.TableAttributes; import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.TableIndexes; -import org.apache.doris.catalog.constraint.Constraint; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Pair; import org.apache.doris.common.io.Text; @@ -82,6 +81,7 @@ public class ExternalTable implements TableIf, Writable, GsonPostProcessable { // dbName is temporarily retained and will be deleted later. To use dbName, please use db.getFullName() @SerializedName(value = "dbName") protected String dbName; + @Deprecated @SerializedName(value = "ta") private final TableAttributes tableAttributes = new TableAttributes(); @@ -226,11 +226,6 @@ public Column getColumn(String name) { return null; } - @Override - public Map getConstraintsMapUnsafe() { - return tableAttributes.getConstraintsMap(); - } - @Override public String getEngine() { return getType().toEngineName(); @@ -549,6 +544,11 @@ public String getRemoteDbName() { return db.getRemoteName(); } + /** + * @deprecated Use ConstraintManager for constraint access. + * This method will be removed in a future version. + */ + @Deprecated public TableAttributes getTableAttributes() { return tableAttributes; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index f6575ac3d45a90..cf0d790354d291 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -126,6 +126,7 @@ import org.apache.doris.event.DropPartitionEvent; import org.apache.doris.foundation.type.ResultOr; import org.apache.doris.info.PartitionNamesInfo; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.mtmv.BaseTableInfo; import org.apache.doris.mtmv.MTMVUtil; import org.apache.doris.mysql.privilege.PrivPredicate; @@ -560,7 +561,8 @@ public void dropDb(String dbName, boolean ifExists, boolean force) throws DdlExc LOG.info("finish drop database[{}], is force : {}", dbName, force); } - public void unprotectDropDb(Database db, boolean isForeDrop, boolean isReplay, long recycleTime) { + public void unprotectDropDb(Database db, boolean isForeDrop, boolean isReplay, long recycleTime) + throws DdlException { for (Table table : db.getTables()) { unprotectDropTable(db, table, isForeDrop, isReplay, recycleTime); } @@ -1028,7 +1030,7 @@ private static String genDropHint(String dbName, TableIf table) { } public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, boolean isReplay, - long recycleTime) { + long recycleTime) throws DdlException { if (table.getType() == TableType.ELASTICSEARCH) { esRepository.deRegisterTable(table.getId()); } @@ -1041,7 +1043,8 @@ public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, Env.getCurrentEnv().getAnalysisManager().removeTableStats(table.getId()); Env.getCurrentEnv().getDictionaryManager().dropTableDictionaries(db.getName(), table.getName()); Env.getCurrentEnv().getQueryStats().clear(Env.getCurrentInternalCatalog().getId(), db.getId(), table.getId()); - table.removeTableIdentifierFromPrimaryTable(); + Env.getCurrentEnv().getConstraintManager().checkAndDropTableConstraints( + new TableNameInfo(table), !isForceDrop && !isReplay); db.unregisterTable(table.getId()); StopWatch watch = StopWatch.createStarted(); Env.getCurrentRecycleBin().recycleTable(db.getId(), table, isReplay, isForceDrop, recycleTime); @@ -1052,7 +1055,7 @@ public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, } private void dropTable(Database db, long tableId, boolean isForceDrop, boolean isReplay, - Long recycleTime) throws MetaNotFoundException { + Long recycleTime) throws MetaNotFoundException, DdlException { Table table = db.getTableOrMetaException(tableId); db.writeLock(); table.writeLock(); @@ -1065,7 +1068,7 @@ private void dropTable(Database db, long tableId, boolean isForceDrop, boolean i } public void replayDropTable(Database db, long tableId, boolean isForceDrop, - Long recycleTime) throws MetaNotFoundException { + Long recycleTime) throws MetaNotFoundException, DdlException { dropTable(db, tableId, isForceDrop, true, recycleTime); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/info/TableNameInfo.java b/fe/fe-core/src/main/java/org/apache/doris/info/TableNameInfo.java index da02d3b58946ce..0ed6bc844ee0f4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/info/TableNameInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/info/TableNameInfo.java @@ -150,6 +150,29 @@ public TableNameInfo(TableIf tableIf) throws AnalysisException { this.tbl = tableName; } + /** + * Create TableNameInfo from a TableIf, returning null if the table + * lacks a database or catalog (e.g., standalone tables in unit tests). + */ + public static TableNameInfo createOrNull(TableIf tableIf) { + if (tableIf == null || StringUtils.isEmpty(tableIf.getName())) { + return null; + } + DatabaseIf db = tableIf.getDatabase(); + if (db == null) { + return null; + } + CatalogIf catalog = db.getCatalog(); + if (catalog == null) { + return null; + } + String tableName = tableIf.getName(); + if (Env.isStoredTableNamesLowerCase()) { + tableName = tableName.toLowerCase(); + } + return new TableNameInfo(catalog.getName(), db.getFullName(), tableName); + } + /** * analyze tableNameInfo * @param ctx ctx @@ -246,9 +269,6 @@ public String toString() { return stringBuilder.toString(); } - /** - * equals - */ @Override public boolean equals(Object o) { if (this == o) { @@ -258,15 +278,13 @@ public boolean equals(Object o) { return false; } TableNameInfo that = (TableNameInfo) o; - return tbl.equals(that.tbl) && db.equals(that.db) && ctl.equals(that.ctl); + return Objects.equals(ctl, that.ctl) && Objects.equals(tbl, that.tbl) + && Objects.equals(db, that.db); } - /** - * hashCode - */ @Override public int hashCode() { - return Objects.hash(tbl, db, ctl); + return Objects.hash(ctl, tbl, db); } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java index 52021622ff8320..4b526691dfa2b3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ForeignKeyContext.java @@ -18,7 +18,11 @@ package org.apache.doris.nereids.rules.rewrite; import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.constraint.ForeignKeyConstraint; +import org.apache.doris.catalog.constraint.PrimaryKeyConstraint; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.NamedExpression; @@ -98,18 +102,28 @@ public Void visitLogicalFilter(LogicalFilter filter, ForeignKeyContext contex } void putAllForeignKeys(TableIf table) { - table.getForeignKeyConstraints().forEach(c -> { + TableNameInfo tableNameInfo = TableNameInfo.createOrNull(table); + if (tableNameInfo == null) { + return; + } + for (ForeignKeyConstraint c : Env.getCurrentEnv().getConstraintManager() + .getForeignKeyConstraints(tableNameInfo)) { Map constraint = c.getForeignToPrimary(table); - constraints.add(c.getForeignToPrimary(table)); + constraints.add(constraint); foreignKeys.addAll(constraint.keySet()); - }); + } } void putAllPrimaryKeys(TableIf table) { - table.getPrimaryKeyConstraints().forEach(c -> { + TableNameInfo tableNameInfo = TableNameInfo.createOrNull(table); + if (tableNameInfo == null) { + return; + } + for (PrimaryKeyConstraint c : Env.getCurrentEnv().getConstraintManager() + .getPrimaryKeyConstraints(tableNameInfo)) { Set primaryKey = c.getPrimaryKeys(table); primaryKeys.addAll(primaryKey); - }); + } } public boolean isForeignKey(Set key) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java index b7b34ed6185531..b66010c9862546 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java @@ -17,9 +17,13 @@ package org.apache.doris.nereids.trees.plans.commands; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.constraint.ForeignKeyConstraint; +import org.apache.doris.catalog.constraint.PrimaryKeyConstraint; +import org.apache.doris.catalog.constraint.UniqueConstraint; import org.apache.doris.common.Pair; -import org.apache.doris.common.util.MetaLockUtils; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.NereidsPlanner; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.properties.PhysicalProperties; @@ -35,12 +39,10 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; +import com.google.common.collect.ImmutableSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import java.util.Comparator; -import java.util.List; import java.util.Set; /** @@ -65,26 +67,28 @@ public AddConstraintCommand(String name, Constraint constraint) { @Override public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { Pair, TableIf> columnsAndTable = extractColumnsAndTable(ctx, constraint.toProject()); - List tables = Lists.newArrayList(columnsAndTable.second); - Pair, TableIf> referencedColumnsAndTable = null; + TableIf table = columnsAndTable.second; + TableNameInfo tableNameInfo = new TableNameInfo(table); + ImmutableList columns = columnsAndTable.first; + if (constraint.isForeignKey()) { - referencedColumnsAndTable = extractColumnsAndTable(ctx, constraint.toReferenceProject()); - tables.add(referencedColumnsAndTable.second); - } - tables.sort((Comparator.comparing(TableIf::getId))); - MetaLockUtils.writeLockTables(tables); - try { - if (constraint.isForeignKey()) { - Preconditions.checkState(referencedColumnsAndTable != null); - columnsAndTable.second.addForeignConstraint(name, columnsAndTable.first, - referencedColumnsAndTable.second, referencedColumnsAndTable.first, false); - } else if (constraint.isPrimaryKey()) { - columnsAndTable.second.addPrimaryKeyConstraint(name, columnsAndTable.first, false); - } else if (constraint.isUnique()) { - columnsAndTable.second.addUniqueConstraint(name, columnsAndTable.first, false); - } - } finally { - MetaLockUtils.writeUnlockTables(tables); + Pair, TableIf> refColumnsAndTable + = extractColumnsAndTable(ctx, constraint.toReferenceProject()); + TableNameInfo refTableInfo = new TableNameInfo(refColumnsAndTable.second); + ForeignKeyConstraint fkConstraint = new ForeignKeyConstraint( + name, columns, refTableInfo, refColumnsAndTable.first); + Env.getCurrentEnv().getConstraintManager().addConstraint( + tableNameInfo, name, fkConstraint, false); + } else if (constraint.isPrimaryKey()) { + PrimaryKeyConstraint pkConstraint = new PrimaryKeyConstraint( + name, ImmutableSet.copyOf(columns)); + Env.getCurrentEnv().getConstraintManager().addConstraint( + tableNameInfo, name, pkConstraint, false); + } else if (constraint.isUnique()) { + UniqueConstraint uniqueConstraint = new UniqueConstraint( + name, ImmutableSet.copyOf(columns)); + Env.getCurrentEnv().getConstraintManager().addConstraint( + tableNameInfo, name, uniqueConstraint, false); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java index 81147411b951b7..b4922b7da2182d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java @@ -17,9 +17,11 @@ package org.apache.doris.nereids.trees.plans.commands; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; -import org.apache.doris.catalog.constraint.Constraint; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.NereidsPlanner; +import org.apache.doris.nereids.analyzer.UnboundRelation; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.properties.PhysicalProperties; import org.apache.doris.nereids.trees.plans.Plan; @@ -56,23 +58,39 @@ public DropConstraintCommand(String name, LogicalPlan plan) { @Override public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { - TableIf table = extractTable(ctx, plan); - table.readLock(); + TableNameInfo tableNameInfo; try { - Constraint constraint = table.getConstraintsMapUnsafe().get(name); - if (constraint == null) { - throw new AnalysisException( - String.format("Unknown constraint %s on table %s.", name, table.getName())); - } - } finally { - table.readUnlock(); + TableIf table = extractTable(ctx, plan); + tableNameInfo = new TableNameInfo(table); + } catch (Exception e) { + // Table may no longer exist (e.g., external table deleted by another system). + // Fall back to extracting the table name from the unresolved plan. + LOG.warn("Table resolution failed for dropping constraint {}, " + + "falling back to name-based lookup: {}", name, e.getMessage()); + tableNameInfo = extractTableNameFromPlan(ctx); } - table.writeLock(); - try { - table.dropConstraint(name, false); - } finally { - table.writeUnlock(); + Env.getCurrentEnv().getConstraintManager().dropConstraint( + tableNameInfo, name, false); + } + + private TableNameInfo extractTableNameFromPlan(ConnectContext ctx) { + if (!(plan instanceof UnboundRelation)) { + throw new AnalysisException( + "Cannot resolve table for dropping constraint " + name); + } + UnboundRelation unbound = (UnboundRelation) plan; + List parts = unbound.getNameParts(); + String ctl = ctx.getCurrentCatalog() != null + ? ctx.getCurrentCatalog().getName() + : "internal"; + String db = ctx.getDatabase(); + // Fill in default catalog/db from connect context if not specified + if (parts.size() == 1) { + return new TableNameInfo(ctl, db, parts.get(0)); + } else if (parts.size() == 2) { + return new TableNameInfo(ctl, parts.get(0), parts.get(1)); } + return new TableNameInfo(parts); } private TableIf extractTable(ConnectContext ctx, LogicalPlan plan) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowConstraintsCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowConstraintsCommand.java index fbe3dfc5261b6f..d24289b0877daa 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowConstraintsCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowConstraintsCommand.java @@ -19,8 +19,11 @@ import org.apache.doris.analysis.StmtType; import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.ScalarType; import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.constraint.Constraint; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; import org.apache.doris.nereids.util.RelationUtil; @@ -32,11 +35,12 @@ import org.apache.hadoop.util.Lists; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; /** - * add constraint command + * show constraints command */ public class ShowConstraintsCommand extends ShowCommand { @@ -65,17 +69,14 @@ public ShowResultSetMetaData getMetaData() { public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor) throws Exception { TableIf tableIf = RelationUtil.getDbAndTable( RelationUtil.getQualifierName(ctx, nameParts), ctx.getEnv(), Optional.empty()).value(); - tableIf.readLock(); - List> res; - try { - res = tableIf.getConstraintsMap().entrySet().stream() - .map(e -> Lists.newArrayList(e.getKey(), - e.getValue().getType().getName(), - e.getValue().toString())) - .collect(Collectors.toList()); - } finally { - tableIf.readUnlock(); - } + TableNameInfo tableNameInfo = new TableNameInfo(tableIf); + Map constraints = Env.getCurrentEnv().getConstraintManager() + .getConstraints(tableNameInfo); + List> res = constraints.entrySet().stream() + .map(e -> Lists.newArrayList(e.getKey(), + e.getValue().getType().getName(), + e.getValue().toString())) + .collect(Collectors.toList()); return new ShowResultSet(META_DATA, res); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java index 96f76726ffa0f3..a0eefef422dff3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCatalogRelation.java @@ -21,12 +21,14 @@ import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.constraint.ConstraintManager; import org.apache.doris.catalog.constraint.PrimaryKeyConstraint; import org.apache.doris.catalog.constraint.TableIdentifier; import org.apache.doris.catalog.constraint.UniqueConstraint; import org.apache.doris.common.IdGenerator; import org.apache.doris.common.util.Util; import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.memo.GroupExpression; import org.apache.doris.nereids.properties.DataTrait; @@ -177,12 +179,17 @@ public List getVirtualColumns() { @Override public void computeUnique(DataTrait.Builder builder) { Set outputSet = Utils.fastToImmutableSet(getOutputSet()); - for (PrimaryKeyConstraint c : table.getPrimaryKeyConstraints()) { + TableNameInfo tableNameInfo = TableNameInfo.createOrNull(table); + if (tableNameInfo == null) { + return; + } + ConstraintManager cm = Env.getCurrentEnv().getConstraintManager(); + for (PrimaryKeyConstraint c : cm.getPrimaryKeyConstraints(tableNameInfo)) { Set columns = c.getPrimaryKeys(table); builder.addUniqueSlot((ImmutableSet) findSlotsByColumn(outputSet, columns)); } - for (UniqueConstraint c : table.getUniqueConstraints()) { + for (UniqueConstraint c : cm.getUniqueConstraints(tableNameInfo)) { Set columns = c.getUniqueKeys(table); builder.addUniqueSlot((ImmutableSet) findSlotsByColumn(outputSet, columns)); } @@ -195,8 +202,13 @@ public void computeUniform(DataTrait.Builder builder) { private ImmutableSet computeFdItems(Set outputSet) { ImmutableSet.Builder builder = ImmutableSet.builder(); + TableNameInfo tableNameInfo = TableNameInfo.createOrNull(table); + if (tableNameInfo == null) { + return builder.build(); + } + ConstraintManager cm = Env.getCurrentEnv().getConstraintManager(); - for (PrimaryKeyConstraint c : table.getPrimaryKeyConstraints()) { + for (PrimaryKeyConstraint c : cm.getPrimaryKeyConstraints(tableNameInfo)) { Set columns = c.getPrimaryKeys(this.getTable()); ImmutableSet slotSet = findSlotsByColumn(outputSet, columns); TableFdItem tableFdItem = FdFactory.INSTANCE.createTableFdItem( @@ -204,7 +216,7 @@ private ImmutableSet computeFdItems(Set outputSet) { builder.add(tableFdItem); } - for (UniqueConstraint c : table.getUniqueConstraints()) { + for (UniqueConstraint c : cm.getUniqueConstraints(tableNameInfo)) { Set columns = c.getUniqueKeys(this.getTable()); boolean allNotNull = true; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCatalogRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCatalogRelation.java index 909ec09ee51537..4937df70ad549b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCatalogRelation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCatalogRelation.java @@ -21,10 +21,12 @@ import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.constraint.ConstraintManager; import org.apache.doris.catalog.constraint.PrimaryKeyConstraint; import org.apache.doris.catalog.constraint.UniqueConstraint; import org.apache.doris.common.IdGenerator; import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.memo.GroupExpression; import org.apache.doris.nereids.processor.post.runtimefilterv2.RuntimeFilterV2; @@ -178,12 +180,17 @@ public String shapeInfo() { @Override public void computeUnique(DataTrait.Builder builder) { Set outputSet = Utils.fastToImmutableSet(getOutputSet()); - for (PrimaryKeyConstraint c : table.getPrimaryKeyConstraints()) { + TableNameInfo tableNameInfo = TableNameInfo.createOrNull(table); + if (tableNameInfo == null) { + return; + } + ConstraintManager cm = Env.getCurrentEnv().getConstraintManager(); + for (PrimaryKeyConstraint c : cm.getPrimaryKeyConstraints(tableNameInfo)) { Set columns = c.getPrimaryKeys(table); builder.addUniqueSlot((ImmutableSet) findSlotsByColumn(outputSet, columns)); } - for (UniqueConstraint c : table.getUniqueConstraints()) { + for (UniqueConstraint c : cm.getUniqueConstraints(tableNameInfo)) { Set columns = c.getUniqueKeys(table); builder.addUniqueSlot((ImmutableSet) findSlotsByColumn(outputSet, columns)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/AlterConstraintLog.java b/fe/fe-core/src/main/java/org/apache/doris/persist/AlterConstraintLog.java index eef8b2cd2b55bc..07c6a0fda3d731 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/AlterConstraintLog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/AlterConstraintLog.java @@ -17,11 +17,12 @@ package org.apache.doris.persist; -import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.constraint.Constraint; import org.apache.doris.catalog.constraint.TableIdentifier; import org.apache.doris.common.io.Text; import org.apache.doris.common.io.Writable; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.persist.gson.GsonPostProcessable; import org.apache.doris.persist.gson.GsonUtils; import com.google.gson.annotations.SerializedName; @@ -30,25 +31,38 @@ import java.io.DataOutput; import java.io.IOException; -public class AlterConstraintLog implements Writable { +/** + * Edit log entry for constraint add/drop operations. + * Uses TableNameInfo (name-based) for persistence. + * Supports backward compatibility with old TableIdentifier (ID-based) format + * via GsonPostProcessable migration. + */ +public class AlterConstraintLog implements Writable, GsonPostProcessable { @SerializedName("ct") final Constraint constraint; + + // Old format: ID-based table identifier (kept for backward compat deserialization) @SerializedName("tid") final TableIdentifier tableIdentifier; - public AlterConstraintLog(Constraint constraint, TableIf table) { - this.constraint = constraint; - this.tableIdentifier = new TableIdentifier(table); - } + // New format: name-based table info + @SerializedName("tni") + private TableNameInfo tableNameInfo; - public TableIf getTableIf() { - return tableIdentifier.toTableIf(); + public AlterConstraintLog(Constraint constraint, TableNameInfo tableNameInfo) { + this.constraint = constraint; + this.tableNameInfo = tableNameInfo; + this.tableIdentifier = null; } public Constraint getConstraint() { return constraint; } + public TableNameInfo getTableNameInfo() { + return tableNameInfo; + } + @Override public void write(DataOutput out) throws IOException { Text.writeString(out, GsonUtils.GSON.toJson(this)); @@ -58,4 +72,19 @@ public static AlterConstraintLog read(DataInput in) throws IOException { String json = Text.readString(in); return GsonUtils.GSON.fromJson(json, AlterConstraintLog.class); } + + @Override + public void gsonPostProcess() throws IOException { + // Migrate from old ID-based format to name-based format + if (tableNameInfo == null && tableIdentifier != null) { + try { + String qualifiedName = tableIdentifier.toQualifiedName(); + if (qualifiedName != null) { + tableNameInfo = new TableNameInfo(qualifiedName); + } + } catch (Exception ignored) { + // Old table may no longer exist; tableNameInfo stays null + } + } + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java index d2760bb7179759..ebc25e53b78f15 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java @@ -41,6 +41,7 @@ import org.apache.doris.catalog.Function; import org.apache.doris.catalog.FunctionSearchDesc; import org.apache.doris.catalog.Resource; +import org.apache.doris.catalog.constraint.Constraint; import org.apache.doris.cloud.CloudWarmUpJob; import org.apache.doris.cloud.catalog.CloudEnv; import org.apache.doris.cloud.persist.CloudMetaSyncPoint; @@ -69,6 +70,7 @@ import org.apache.doris.ha.MasterInfo; import org.apache.doris.indexpolicy.DropIndexPolicyLog; import org.apache.doris.indexpolicy.IndexPolicy; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.insertoverwrite.InsertOverwriteLog; import org.apache.doris.job.base.AbstractJob; import org.apache.doris.journal.Journal; @@ -1191,18 +1193,36 @@ public static void loadJournal(Env env, Long logId, JournalEntity journal) { case OperationType.OP_ADD_CONSTRAINT: { final AlterConstraintLog log = (AlterConstraintLog) journal.getData(); try { - log.getTableIf().replayAddConstraint(log.getConstraint()); + TableNameInfo tni = log.getTableNameInfo(); + Constraint constraint = log.getConstraint(); + if (tni == null) { + LOG.warn("Failed to replay add constraint {}: " + + "table name could not be resolved", + constraint.getName()); + break; + } + env.getConstraintManager().addConstraint( + tni, constraint.getName(), constraint, true); } catch (Exception e) { - LOG.error("Failed to replay add constraint", e); + LOG.warn("Failed to replay add constraint", e); } break; } case OperationType.OP_DROP_CONSTRAINT: { final AlterConstraintLog log = (AlterConstraintLog) journal.getData(); try { - log.getTableIf().replayDropConstraint(log.getConstraint().getName()); + TableNameInfo tni = log.getTableNameInfo(); + Constraint constraint = log.getConstraint(); + if (tni == null) { + LOG.warn("Failed to replay drop constraint {}: " + + "table name could not be resolved", + constraint.getName()); + break; + } + env.getConstraintManager().dropConstraint( + tni, constraint.getName(), true); } catch (Exception e) { - LOG.error("Failed to replay drop constraint", e); + LOG.warn("Failed to replay drop constraint", e); } break; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/meta/MetaPersistMethod.java b/fe/fe-core/src/main/java/org/apache/doris/persist/meta/MetaPersistMethod.java index 6ed7645d4610b6..d0f6faaeab1e84 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/meta/MetaPersistMethod.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/meta/MetaPersistMethod.java @@ -292,6 +292,12 @@ public static MetaPersistMethod create(String name) throws NoSuchMethodException metaPersistMethod.writeMethod = Env.class.getDeclaredMethod("saveLanceIndexJobManager", CountingDataOutputStream.class, long.class); break; + case "constraintManager": + metaPersistMethod.readMethod = Env.class.getDeclaredMethod("loadConstraintManager", + DataInputStream.class, long.class); + metaPersistMethod.writeMethod = Env.class.getDeclaredMethod("saveConstraintManager", + CountingDataOutputStream.class, long.class); + break; default: break; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/meta/PersistMetaModules.java b/fe/fe-core/src/main/java/org/apache/doris/persist/meta/PersistMetaModules.java index 8f8f00dd64ea2e..1202531bbd76e0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/meta/PersistMetaModules.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/meta/PersistMetaModules.java @@ -44,7 +44,7 @@ public class PersistMetaModules { "globalFunction", "workloadGroups", "binlogs", "resourceGroups", "AnalysisMgrV2", "AsyncJobManager", "workloadSchedPolicy", "insertOverwrite", "plsql", "dictionaryManager", "indexPolicy", "KeyManagerStore", - "authenticationIntegrations", "roleMappings", "lanceIndexJobManager" + "authenticationIntegrations", "roleMappings", "lanceIndexJobManager", "constraintManager" ); // The modules in `CloudEnv`. diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java new file mode 100644 index 00000000000000..456557c87d57c8 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java @@ -0,0 +1,587 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog.constraint; + +import org.apache.doris.common.DdlException; +import org.apache.doris.info.TableNameInfo; +import org.apache.doris.nereids.exceptions.AnalysisException; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInput; +import java.io.DataInputStream; +import java.io.DataOutput; +import java.io.DataOutputStream; +import java.util.Map; + +/** + * Unit tests for ConstraintManager, testing direct API methods + * without requiring a full FE environment. + * All mutations use replay=true to bypass table validation. + */ +class ConstraintManagerTest { + private ConstraintManager mgr; + + private static final TableNameInfo T1 = new TableNameInfo("ctl", "db", "t1"); + private static final TableNameInfo T2 = new TableNameInfo("ctl", "db", "t2"); + private static final TableNameInfo T3 = new TableNameInfo("ctl", "db", "t3"); + + @BeforeEach + void setUp() { + mgr = new ConstraintManager(); + } + + // ==================== isEmpty ==================== + + @Test + void isEmptyOnNewManager() { + Assertions.assertTrue(mgr.isEmpty()); + } + + @Test + void isEmptyAfterAdd() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Assertions.assertFalse(mgr.isEmpty()); + } + + @Test + void isEmptyAfterAddAndDrop() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.dropConstraint(T1, "pk", true); + Assertions.assertTrue(mgr.isEmpty()); + } + + // ==================== addConstraint / getConstraint ==================== + + @Test + void addAndGetPrimaryKey() { + PrimaryKeyConstraint pk = newPk("pk", "k1"); + mgr.addConstraint(T1, "pk", pk, true); + Assertions.assertSame(pk, mgr.getConstraint(T1, "pk")); + } + + @Test + void addAndGetUniqueKey() { + UniqueConstraint uk = new UniqueConstraint("uk", ImmutableSet.of("c1")); + mgr.addConstraint(T1, "uk", uk, true); + Assertions.assertSame(uk, mgr.getConstraint(T1, "uk")); + } + + @Test + void addDuplicateConstraintThrows() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Assertions.assertThrows(AnalysisException.class, + () -> mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true)); + } + + @Test + void getConstraintNonExistentTableReturnsNull() { + Assertions.assertNull(mgr.getConstraint(T1, "anything")); + } + + @Test + void getConstraintNonExistentNameReturnsNull() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Assertions.assertNull(mgr.getConstraint(T1, "nonexistent")); + } + + // ==================== getConstraints ==================== + + @Test + void getConstraintsReturnsImmutableCopy() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Map result = mgr.getConstraints(T1); + Assertions.assertEquals(1, result.size()); + Assertions.assertThrows(UnsupportedOperationException.class, + () -> result.put("x", newPk("x", "x"))); + } + + @Test + void getConstraintsForNonExistentTableReturnsEmpty() { + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + } + + // ==================== Type-specific getters ==================== + + @Test + void getPrimaryKeyConstraints() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + mgr.addConstraint(T2, "pk2", newPk("pk2", "k1"), true); + mgr.addConstraint(T1, "fk", newFk("fk", T2, "c1", "k1"), true); + + Assertions.assertEquals(1, mgr.getPrimaryKeyConstraints(T1).size()); + Assertions.assertInstanceOf(PrimaryKeyConstraint.class, + mgr.getPrimaryKeyConstraints(T1).get(0)); + } + + @Test + void getForeignKeyConstraints() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "pk2", newPk("pk2", "k1"), true); + mgr.addConstraint(T1, "fk", newFk("fk", T2, "c1", "k1"), true); + + Assertions.assertEquals(1, mgr.getForeignKeyConstraints(T1).size()); + Assertions.assertInstanceOf(ForeignKeyConstraint.class, + mgr.getForeignKeyConstraints(T1).get(0)); + } + + @Test + void getUniqueConstraints() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + + Assertions.assertEquals(1, mgr.getUniqueConstraints(T1).size()); + Assertions.assertInstanceOf(UniqueConstraint.class, + mgr.getUniqueConstraints(T1).get(0)); + } + + @Test + void typeSpecificGettersReturnEmptyForUnknownTable() { + Assertions.assertTrue(mgr.getPrimaryKeyConstraints(T1).isEmpty()); + Assertions.assertTrue(mgr.getForeignKeyConstraints(T1).isEmpty()); + Assertions.assertTrue(mgr.getUniqueConstraints(T1).isEmpty()); + } + + // ==================== dropConstraint ==================== + + @Test + void dropConstraintRemoves() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.dropConstraint(T1, "pk", true); + Assertions.assertNull(mgr.getConstraint(T1, "pk")); + } + + @Test + void dropNonExistentConstraintThrowsInNonReplay() { + Assertions.assertThrows(AnalysisException.class, + () -> mgr.dropConstraint(T1, "missing", false)); + } + + @Test + void dropNonExistentConstraintSilentInReplay() { + // Should not throw + mgr.dropConstraint(T1, "missing", true); + } + + // ==================== FK bidirectional references ==================== + + @Test + void addForeignKeyRegistersBidirectionalReference() { + PrimaryKeyConstraint pk = newPk("pk", "k1"); + mgr.addConstraint(T1, "pk", pk, true); + ForeignKeyConstraint fk = newFk("fk", T1, "c1", "k1"); + mgr.addConstraint(T2, "fk", fk, true); + + // PK on T1 should have T2 in its foreign table list + PrimaryKeyConstraint loadedPk = (PrimaryKeyConstraint) mgr.getConstraint(T1, "pk"); + Assertions.assertTrue(loadedPk.getForeignTableInfos().stream() + .anyMatch(t -> t.getTbl().equals("t2"))); + } + + @Test + void dropForeignKeyRemovesBidirectionalReference() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + mgr.dropConstraint(T2, "fk", true); + + PrimaryKeyConstraint loadedPk = (PrimaryKeyConstraint) mgr.getConstraint(T1, "pk"); + Assertions.assertTrue(loadedPk.getForeignTableInfos().isEmpty()); + } + + @Test + void dropPrimaryKeyCascadesDropForeignKeys() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk1", newFk("fk1", T1, "c1", "k1"), true); + mgr.addConstraint(T3, "fk2", newFk("fk2", T1, "c1", "k1"), true); + + mgr.dropConstraint(T1, "pk", true); + + // FK on T2 and T3 should also be removed + Assertions.assertTrue(mgr.getConstraints(T2).isEmpty()); + Assertions.assertTrue(mgr.getConstraints(T3).isEmpty()); + } + + // ==================== dropTableConstraints ==================== + + @Test + void dropTableConstraintsRemovesAll() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + mgr.dropTableConstraints(T1); + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + } + + @Test + void dropTableConstraintsCascadesFKReferences() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + // Drop T1's constraints → PK dropped → FK on T2 cascade-dropped + mgr.dropTableConstraints(T1); + Assertions.assertTrue(mgr.getConstraints(T2).isEmpty()); + } + + @Test + void dropTableConstraintsOnNonExistentTableIsNoop() { + // Should not throw + mgr.dropTableConstraints(new TableNameInfo("x", "y", "z")); + } + + // ==================== checkAndDropTableConstraints ==================== + + @Test + void checkAndDropBlocksWhenFKExists() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + Assertions.assertThrows(DdlException.class, + () -> mgr.checkAndDropTableConstraints(T1, true)); + // Constraints should still be intact + Assertions.assertNotNull(mgr.getConstraint(T1, "pk")); + } + + @Test + void checkAndDropWithoutCheckDropsEvenWithFK() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + Assertions.assertDoesNotThrow( + () -> mgr.checkAndDropTableConstraints(T1, false)); + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + Assertions.assertTrue(mgr.getConstraints(T2).isEmpty()); + } + + @Test + void checkAndDropSucceedsWhenNoFK() throws DdlException { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.checkAndDropTableConstraints(T1, true); + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + } + + // ==================== checkNoReferencingForeignKeys ==================== + + @Test + void checkNoReferencingForeignKeysPassesWithoutFK() throws DdlException { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.checkNoReferencingForeignKeys(T1); // no exception + } + + @Test + void checkNoReferencingForeignKeysThrowsWithFK() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + Assertions.assertThrows(DdlException.class, + () -> mgr.checkNoReferencingForeignKeys(T1)); + } + + @Test + void checkNoReferencingForeignKeysOnEmptyTableIsNoop() throws DdlException { + mgr.checkNoReferencingForeignKeys(T1); // no exception + } + + // ==================== findConstraintWithColumn ==================== + + @Test + void findConstraintWithColumnFindsPK() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Assertions.assertEquals("pk", mgr.findConstraintWithColumn(T1, "k1")); + } + + @Test + void findConstraintWithColumnFindsUnique() { + mgr.addConstraint(T1, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + Assertions.assertEquals("uk", mgr.findConstraintWithColumn(T1, "c1")); + } + + @Test + void findConstraintWithColumnFindsFK() { + mgr.addConstraint(T2, "pk2", newPk("pk2", "k1"), true); + mgr.addConstraint(T1, "fk", newFk("fk", T2, "c1", "k1"), true); + Assertions.assertEquals("fk", mgr.findConstraintWithColumn(T1, "c1")); + } + + @Test + void findConstraintWithColumnReturnsNullForUnknownColumn() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + Assertions.assertNull(mgr.findConstraintWithColumn(T1, "nonexistent")); + } + + @Test + void findConstraintWithColumnReturnsNullForUnknownTable() { + Assertions.assertNull(mgr.findConstraintWithColumn(T1, "k1")); + } + + // ==================== dropCatalogConstraints ==================== + + @Test + void dropCatalogConstraintsRemovesOnlyMatchingCatalog() { + TableNameInfo extT1 = new TableNameInfo("extCtl", "db", "t1"); + TableNameInfo extT2 = new TableNameInfo("extCtl", "db", "t2"); + mgr.addConstraint(extT1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(extT2, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + + mgr.dropCatalogConstraints("extCtl"); + + Assertions.assertTrue(mgr.getConstraints(extT1).isEmpty()); + Assertions.assertTrue(mgr.getConstraints(extT2).isEmpty()); + // T1 is in "ctl" catalog — should be unaffected + Assertions.assertNotNull(mgr.getConstraint(T1, "pk")); + } + + @Test + void dropCatalogConstraintsCascadesFKsAcrossCatalogs() { + TableNameInfo extT = new TableNameInfo("extCtl", "db", "t1"); + // PK on extT, FK on T1 referencing extT + mgr.addConstraint(extT, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "fk", newFk("fk", extT, "c1", "k1"), true); + + mgr.dropCatalogConstraints("extCtl"); + + Assertions.assertTrue(mgr.getConstraints(extT).isEmpty()); + // FK on T1 referencing extT is cascade-dropped because the referenced PK was removed + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty(), + "FK on T1 should be cascade-dropped when referenced PK's catalog is dropped"); + } + + @Test + void dropCatalogConstraintsOnNonExistentCatalogIsNoop() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.dropCatalogConstraints("nonExistent"); + Assertions.assertNotNull(mgr.getConstraint(T1, "pk")); + } + + // ==================== renameTable ==================== + + @Test + void renameTableMovesConstraints() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + TableNameInfo newT = new TableNameInfo("ctl", "db", "t1_renamed"); + mgr.renameTable(T1, newT); + + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + Assertions.assertNotNull(mgr.getConstraint(newT, "pk")); + } + + @Test + void renameTableUpdatesFKReferencesInOtherTables() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + + TableNameInfo newT1 = new TableNameInfo("ctl", "db", "t1_renamed"); + mgr.renameTable(T1, newT1); + + ForeignKeyConstraint fk = (ForeignKeyConstraint) mgr.getConstraint(T2, "fk"); + Assertions.assertEquals("t1_renamed", fk.getReferencedTableName().getTbl()); + } + + @Test + void renameTableUpdatesPKForeignTableListInOtherTables() { + // T2 has PK, T1 has FK referencing T2. Rename T1. + mgr.addConstraint(T2, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "fk", newFk("fk", T2, "c1", "k1"), true); + + TableNameInfo newT1 = new TableNameInfo("ctl", "db", "t1_renamed"); + mgr.renameTable(T1, newT1); + + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) mgr.getConstraint(T2, "pk"); + Assertions.assertTrue(pk.getForeignTableInfos().stream() + .anyMatch(t -> t.getTbl().equals("t1_renamed"))); + Assertions.assertFalse(pk.getForeignTableInfos().stream() + .anyMatch(t -> t.getTbl().equals("t1"))); + } + + @Test + void renameNonExistentTableIsNoop() { + TableNameInfo ghost = new TableNameInfo("ctl", "db", "ghost"); + TableNameInfo newGhost = new TableNameInfo("ctl", "db", "ghost2"); + mgr.renameTable(ghost, newGhost); // should not throw + Assertions.assertTrue(mgr.isEmpty()); + } + + // ==================== swapTableConstraints ==================== + + @Test + void swapTableConstraintsExchangesMappings() { + mgr.addConstraint(T1, "pk1", newPk("pk1", "k1"), true); + mgr.addConstraint(T2, "uk2", new UniqueConstraint("uk2", ImmutableSet.of("c1")), true); + + mgr.swapTableConstraints(T1, T2); + + // pk1 should now be under T2 + Assertions.assertNotNull(mgr.getConstraint(T2, "pk1")); + // uk2 should now be under T1 + Assertions.assertNotNull(mgr.getConstraint(T1, "uk2")); + } + + @Test + void swapTableConstraintsWhenOneSideEmpty() { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + + mgr.swapTableConstraints(T1, T2); + + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + Assertions.assertNotNull(mgr.getConstraint(T2, "pk")); + } + + @Test + void swapTableConstraintsUpdatesFKReferences() { + // T1 has PK, T3 has FK referencing T1. Swap T1 and T2. + mgr.addConstraint(T1, "pk1", newPk("pk1", "k1"), true); + mgr.addConstraint(T2, "uk2", new UniqueConstraint("uk2", ImmutableSet.of("c1")), true); + mgr.addConstraint(T3, "fk", newFk("fk", T1, "c1", "k1"), true); + + mgr.swapTableConstraints(T1, T2); + + // T3's FK should now reference T2 (was T1) + ForeignKeyConstraint fk = (ForeignKeyConstraint) mgr.getConstraint(T3, "fk"); + Assertions.assertEquals("t2", fk.getReferencedTableName().getTbl()); + } + + // ==================== dropAndRenameConstraints ==================== + + @Test + void dropAndRenameDropsOldAndMovesNew() { + mgr.addConstraint(T1, "pk_old", newPk("pk_old", "k1"), true); + mgr.addConstraint(T2, "pk_new", newPk("pk_new", "k2"), true); + + mgr.dropAndRenameConstraints(T1, T2); + + // T2's constraints should now be under T1's key + Assertions.assertNotNull(mgr.getConstraint(T1, "pk_new")); + // T1's old constraint should be gone + Assertions.assertNull(mgr.getConstraint(T1, "pk_old")); + // T2 should have no constraints + Assertions.assertTrue(mgr.getConstraints(T2).isEmpty()); + } + + @Test + void dropAndRenameUpdatesFKReferences() { + // T2 has PK, T3 has FK referencing T2. Replace T1 with T2 (no swap). + mgr.addConstraint(T2, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T3, "fk", newFk("fk", T2, "c1", "k1"), true); + + mgr.dropAndRenameConstraints(T1, T2); + + // T3's FK should now reference T1 (was T2, since T2 was renamed to T1) + ForeignKeyConstraint fk = (ForeignKeyConstraint) mgr.getConstraint(T3, "fk"); + Assertions.assertEquals("t1", fk.getReferencedTableName().getTbl()); + } + + // ==================== migrateFromTable ==================== + + @Test + void migrateFromTableAddsConstraints() { + PrimaryKeyConstraint pk = newPk("pk", "k1"); + Map existing = ImmutableMap.of("pk", pk); + mgr.migrateFromTable(T1, existing); + Assertions.assertSame(pk, mgr.getConstraint(T1, "pk")); + } + + @Test + void migrateFromTableWithEmptyMapIsNoop() { + mgr.migrateFromTable(T1, ImmutableMap.of()); + Assertions.assertTrue(mgr.isEmpty()); + } + + @Test + void migrateFromTableWithNullIsNoop() { + mgr.migrateFromTable(T1, null); + Assertions.assertTrue(mgr.isEmpty()); + } + + // ==================== rebuildForeignKeyReferences ==================== + + @Test + void rebuildForeignKeyReferencesWiresFKToPK() { + // Simulate migration: PK on T1, FK on T2 referencing T1, + // but PK doesn't know about T2 yet (as during per-table migration) + PrimaryKeyConstraint pk = newPk("pk", "k1"); + mgr.addConstraint(T1, "pk", pk, true); + // Add FK without registering bidirectional reference + ForeignKeyConstraint fk = newFk("fk", T1, "c1", "k1"); + Map t2Map = new java.util.HashMap<>(); + t2Map.put("fk", fk); + mgr.migrateFromTable(T2, t2Map); + + // Before rebuild: PK doesn't know about T2 + PrimaryKeyConstraint pkBefore = (PrimaryKeyConstraint) mgr.getConstraint(T1, "pk"); + Assertions.assertTrue(pkBefore.getForeignTableInfos().isEmpty(), + "Before rebuild, PK should not know about T2"); + + // After rebuild: PK should know about T2 + mgr.rebuildForeignKeyReferences(); + + PrimaryKeyConstraint loadedPk = (PrimaryKeyConstraint) mgr.getConstraint(T1, "pk"); + Assertions.assertFalse(loadedPk.getForeignTableInfos().isEmpty()); + } + + // ==================== Serialization ==================== + + @Test + void writeAndReadRoundTrip() throws Exception { + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T1, "uk", new UniqueConstraint("uk", ImmutableSet.of("c1")), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + DataOutput out = new DataOutputStream(baos); + mgr.write(out); + + DataInput in = new DataInputStream( + new ByteArrayInputStream(baos.toByteArray())); + ConstraintManager loaded = ConstraintManager.read(in); + + Assertions.assertEquals(2, loaded.getConstraints(T1).size()); + Assertions.assertEquals(1, loaded.getConstraints(T2).size()); + Assertions.assertInstanceOf(PrimaryKeyConstraint.class, + loaded.getConstraint(T1, "pk")); + Assertions.assertInstanceOf(ForeignKeyConstraint.class, + loaded.getConstraint(T2, "fk")); + } + + @Test + void writeAndReadEmptyManager() throws Exception { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + DataOutput out = new DataOutputStream(baos); + mgr.write(out); + + DataInput in = new DataInputStream( + new ByteArrayInputStream(baos.toByteArray())); + ConstraintManager loaded = ConstraintManager.read(in); + + Assertions.assertTrue(loaded.isEmpty()); + } + + // ==================== Helpers ==================== + + private static PrimaryKeyConstraint newPk(String name, String... columns) { + return new PrimaryKeyConstraint(name, ImmutableSet.copyOf(columns)); + } + + private static ForeignKeyConstraint newFk(String name, TableNameInfo refTable, + String fkCol, String pkCol) { + return new ForeignKeyConstraint(name, + ImmutableList.of(fkCol), refTable, ImmutableList.of(pkCol)); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java index e33b4ae417d953..d796ffa8c9a480 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java @@ -20,12 +20,11 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.PrimitiveType; -import org.apache.doris.catalog.Table; import org.apache.doris.catalog.TableIf; import org.apache.doris.common.Config; import org.apache.doris.common.FeConstants; -import org.apache.doris.datasource.ExternalTable; import org.apache.doris.datasource.test.TestExternalCatalog; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.journal.JournalEntity; import org.apache.doris.nereids.util.PlanPatternMatchSupported; import org.apache.doris.nereids.util.RelationUtil; @@ -34,7 +33,6 @@ import org.apache.doris.persist.OperationType; import org.apache.doris.utframe.TestWithFeService; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import org.junit.jupiter.api.Assertions; @@ -88,17 +86,25 @@ void addConstraintLogPersistTest() throws Exception { TableIf tableIf = RelationUtil.getTable( RelationUtil.getQualifierName(connectContext, Lists.newArrayList("test", "t1")), connectContext.getEnv(), Optional.empty()); - Map constraintMap = tableIf.getConstraintsMap(); - tableIf.getConstraintsMapUnsafe().clear(); - Assertions.assertTrue(tableIf.getConstraintsMap().isEmpty()); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + TableNameInfo tni = new TableNameInfo(qualifiedName); + ConstraintManager mgr = Env.getCurrentEnv().getConstraintManager(); + Map constraintMap = mgr.getConstraints(tni); + // Clear constraints in manager to test replay + mgr.dropConstraint(tni, "fk", true); + mgr.dropConstraint(tni, "uk", true); + mgr.dropConstraint(tni, "pk", true); + Assertions.assertTrue(mgr.getConstraints(tni).isEmpty()); + // Write constraints as editlog entries ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); for (Constraint value : constraintMap.values()) { JournalEntity journalEntity = new JournalEntity(); - journalEntity.setData(new AlterConstraintLog(value, tableIf)); + journalEntity.setData(new AlterConstraintLog(value, new TableNameInfo(qualifiedName))); journalEntity.setOpCode(OperationType.OP_ADD_CONSTRAINT); journalEntity.write(output); } + // Replay from editlog InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); for (int i = 0; i < constraintMap.values().size(); i++) { @@ -106,7 +112,7 @@ void addConstraintLogPersistTest() throws Exception { journalEntity.readFields(input); EditLog.loadJournal(Env.getCurrentEnv(), 0L, journalEntity); } - Assertions.assertEquals(tableIf.getConstraintsMap(), constraintMap); + Assertions.assertEquals(mgr.getConstraints(tni).size(), constraintMap.size()); dropConstraint("alter table t1 drop constraint fk"); dropConstraint("alter table t1 drop constraint pk"); dropConstraint("alter table t2 drop constraint pk"); @@ -123,15 +129,20 @@ void dropConstraintLogPersistTest() throws Exception { TableIf tableIf = RelationUtil.getTable( RelationUtil.getQualifierName(connectContext, Lists.newArrayList("test", "t1")), connectContext.getEnv(), Optional.empty()); - Map constraintMap = tableIf.getConstraintsMap(); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + TableNameInfo tni = new TableNameInfo(qualifiedName); + ConstraintManager mgr = Env.getCurrentEnv().getConstraintManager(); + Map constraintMap = mgr.getConstraints(tni); + // Write drop entries for each constraint ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); for (Constraint value : constraintMap.values()) { JournalEntity journalEntity = new JournalEntity(); - journalEntity.setData(new AlterConstraintLog(value, tableIf)); + journalEntity.setData(new AlterConstraintLog(value, new TableNameInfo(qualifiedName))); journalEntity.setOpCode(OperationType.OP_DROP_CONSTRAINT); journalEntity.write(output); } + // Replay drops InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); for (int i = 0; i < constraintMap.values().size(); i++) { @@ -139,7 +150,9 @@ void dropConstraintLogPersistTest() throws Exception { journalEntity.readFields(input); EditLog.loadJournal(Env.getCurrentEnv(), 0L, journalEntity); } - Assertions.assertTrue(tableIf.getConstraintsMap().isEmpty()); + Assertions.assertTrue(mgr.getConstraints(tni).isEmpty()); + // Clean up t2 pk + dropConstraint("alter table t2 drop constraint pk"); } @Test @@ -151,13 +164,20 @@ void constraintWithTablePersistTest() throws Exception { TableIf tableIf = RelationUtil.getTable( RelationUtil.getQualifierName(connectContext, Lists.newArrayList("test", "t1")), connectContext.getEnv(), Optional.empty()); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + TableNameInfo tni = new TableNameInfo(qualifiedName); + ConstraintManager mgr = Env.getCurrentEnv().getConstraintManager(); + Map constraintMap = mgr.getConstraints(tni); + Assertions.assertEquals(3, constraintMap.size()); + // Test ConstraintManager serialization/deserialization ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); - tableIf.write(output); + mgr.write(output); InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); - TableIf loadTable = Table.read(input); - Assertions.assertEquals(loadTable.getConstraintsMap(), tableIf.getConstraintsMap()); + ConstraintManager loadedMgr = ConstraintManager.read(input); + Assertions.assertEquals(loadedMgr.getConstraints(tni).size(), + constraintMap.size()); dropConstraint("alter table t1 drop constraint fk"); dropConstraint("alter table t1 drop constraint pk"); dropConstraint("alter table t2 drop constraint pk"); @@ -166,19 +186,19 @@ void constraintWithTablePersistTest() throws Exception { @Test void externalTableTest() throws Exception { - ExternalTable externalTable = new ExternalTable(); - try { - externalTable.addPrimaryKeyConstraint("pk", ImmutableList.of("col"), false); - } catch (Exception ignore) { - // ignore - } + // Test ConstraintManager serialization with manually added constraints + ConstraintManager mgr = new ConstraintManager(); + PrimaryKeyConstraint pk = new PrimaryKeyConstraint("pk", + com.google.common.collect.ImmutableSet.of("col")); + TableNameInfo extTni = new TableNameInfo("test.db.extTable"); + mgr.addConstraint(extTni, "pk", pk, true); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); - externalTable.write(output); + mgr.write(output); InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); - TableIf loadTable = ExternalTable.read(input); - Assertions.assertEquals(1, loadTable.getConstraintsMap().size()); + ConstraintManager loadedMgr = ConstraintManager.read(input); + Assertions.assertEquals(1, loadedMgr.getConstraints(extTni).size()); } @Test @@ -194,26 +214,29 @@ void addConstraintLogPersistForExternalTableTest() throws Exception { TableIf tableIf = RelationUtil.getTable( RelationUtil.getQualifierName(connectContext, Lists.newArrayList("extCtl1", "db1", "tbl11")), connectContext.getEnv(), Optional.empty()); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + TableNameInfo tni = new TableNameInfo(qualifiedName); + ConstraintManager mgr = Env.getCurrentEnv().getConstraintManager(); // add constraints addConstraint("alter table extCtl1.db1.tbl11 add constraint pk primary key (a11)"); addConstraint("alter table extCtl1.db1.tbl11 add constraint uk unique (a11)"); - Assertions.assertEquals(2, tableIf.getConstraintsMap().size()); - // clear the constraints - Map constraintMap = tableIf.getConstraintsMap(); - // save constraints map in edit log + Assertions.assertEquals(2, mgr.getConstraints(tni).size()); + // save constraints in edit log format + Map constraintMap = mgr.getConstraints(tni); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); for (Constraint value : new ArrayList<>(constraintMap.values())) { JournalEntity journalEntity = new JournalEntity(); - journalEntity.setData(new AlterConstraintLog(value, tableIf)); + journalEntity.setData(new AlterConstraintLog(value, new TableNameInfo(qualifiedName))); journalEntity.setOpCode(OperationType.OP_ADD_CONSTRAINT); journalEntity.write(output); } - // clear constraints map manually - tableIf.getConstraintsMapUnsafe().clear(); - Assertions.assertTrue(tableIf.getConstraintsMap().isEmpty()); - // add constraints back from edit log + // Clear constraints to test replay + mgr.dropConstraint(tni, "pk", true); + mgr.dropConstraint(tni, "uk", true); + Assertions.assertTrue(mgr.getConstraints(tni).isEmpty()); + // Replay from editlog InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); for (int i = 0; i < constraintMap.values().size(); i++) { @@ -221,7 +244,7 @@ void addConstraintLogPersistForExternalTableTest() throws Exception { journalEntity.readFields(input); EditLog.loadJournal(Env.getCurrentEnv(), 0L, journalEntity); } - Assertions.assertEquals(2, tableIf.getConstraintsMap().size()); + Assertions.assertEquals(2, mgr.getConstraints(tni).size()); } @Test @@ -237,23 +260,25 @@ void dropConstraintLogPersistForExternalTest() throws Exception { TableIf tableIf = RelationUtil.getTable( RelationUtil.getQualifierName(connectContext, Lists.newArrayList("extCtl2", "db1", "tbl11")), connectContext.getEnv(), Optional.empty()); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + TableNameInfo tni = new TableNameInfo(qualifiedName); + ConstraintManager mgr = Env.getCurrentEnv().getConstraintManager(); // add constraints addConstraint("alter table extCtl2.db1.tbl11 add constraint pk primary key (a11)"); addConstraint("alter table extCtl2.db1.tbl11 add constraint uk unique (a11)"); - Assertions.assertEquals(2, tableIf.getConstraintsMap().size()); - // drop it - // dropConstraint("alter table extCtl2.db1.tbl11 drop constraint pk"); - // dropConstraint("alter table extCtl2.db1.tbl11 drop constraint uk"); - Map constraintMap = tableIf.getConstraintsMap(); + Assertions.assertEquals(2, mgr.getConstraints(tni).size()); + // Write drop editlog entries + Map constraintMap = mgr.getConstraints(tni); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(outputStream); for (Constraint value : constraintMap.values()) { JournalEntity journalEntity = new JournalEntity(); - journalEntity.setData(new AlterConstraintLog(value, tableIf)); + journalEntity.setData(new AlterConstraintLog(value, new TableNameInfo(qualifiedName))); journalEntity.setOpCode(OperationType.OP_DROP_CONSTRAINT); journalEntity.write(output); } + // Replay drops InputStream inputStream = new ByteArrayInputStream(outputStream.toByteArray()); DataInput input = new DataInputStream(inputStream); for (int i = 0; i < constraintMap.values().size(); i++) { @@ -261,11 +286,44 @@ void dropConstraintLogPersistForExternalTest() throws Exception { journalEntity.readFields(input); EditLog.loadJournal(Env.getCurrentEnv(), 0L, journalEntity); } - Assertions.assertTrue(tableIf.getConstraintsMap().isEmpty()); + Assertions.assertTrue(mgr.getConstraints(tni).isEmpty()); Env.getCurrentEnv().changeCatalog(connectContext, "internal"); } + @Test + void backwardCompatAlterConstraintLogTest() throws Exception { + // Simulate old-format AlterConstraintLog that only has TableIdentifier (no TableNameInfo) + TableIf tableIf = RelationUtil.getTable( + RelationUtil.getQualifierName(connectContext, Lists.newArrayList("test", "t1")), + connectContext.getEnv(), Optional.empty()); + String qualifiedName = tableIf.getNameWithFullQualifiers(); + + // Build old-format JSON manually with only "tid" (TableIdentifier) and "ct" (Constraint) + long catalogId = tableIf.getDatabase().getCatalog().getId(); + long dbId = tableIf.getDatabase().getId(); + long tableId = tableIf.getId(); + PrimaryKeyConstraint pk = new PrimaryKeyConstraint("pk_compat", + com.google.common.collect.ImmutableSet.of("k1")); + String pkJson = org.apache.doris.persist.gson.GsonUtils.GSON.toJson(pk); + String oldFormatJson = "{\"ct\":" + pkJson + + ",\"tid\":{\"cId\":" + catalogId + + ",\"dbId\":" + dbId + + ",\"tId\":" + tableId + "}}"; + + // Deserialize using GsonUtils (should trigger gsonPostProcess) + AlterConstraintLog log = org.apache.doris.persist.gson.GsonUtils.GSON + .fromJson(oldFormatJson, AlterConstraintLog.class); + + // Verify gsonPostProcess migrated TableIdentifier -> TableNameInfo + TableNameInfo tni = log.getTableNameInfo(); + Assertions.assertNotNull(tni, + "gsonPostProcess should have migrated TableIdentifier to TableNameInfo"); + String resolvedName = tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl(); + Assertions.assertEquals(qualifiedName, resolvedName); + Assertions.assertEquals("pk_compat", log.getConstraint().getName()); + } + public static class RefreshCatalogProvider implements TestExternalCatalog.TestCatalogProvider { public static final Map>> MOCKED_META; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/ConstraintTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/ConstraintTest.java index c0ea76436972b5..6cb5bc83863e6d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/ConstraintTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/ConstraintTest.java @@ -18,11 +18,13 @@ package org.apache.doris.nereids.trees.plans; import org.apache.doris.catalog.Column; -import org.apache.doris.catalog.TableIf; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.constraint.Constraint; +import org.apache.doris.catalog.constraint.ConstraintManager; import org.apache.doris.catalog.constraint.ForeignKeyConstraint; import org.apache.doris.catalog.constraint.PrimaryKeyConstraint; import org.apache.doris.catalog.constraint.UniqueConstraint; +import org.apache.doris.info.TableNameInfo; import org.apache.doris.nereids.parser.NereidsParser; import org.apache.doris.nereids.trees.expressions.SlotReference; import org.apache.doris.nereids.trees.plans.commands.AddConstraintCommand; @@ -35,10 +37,14 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.List; import java.util.Set; class ConstraintTest extends TestWithFeService implements PlanPatternMatchSupported { + + private ConstraintManager getConstraintMgr() { + return Env.getCurrentEnv().getConstraintManager(); + } + @Override public void runBeforeAll() throws Exception { createDatabase("test"); @@ -78,7 +84,8 @@ void primaryKeyConstraintTest() throws Exception { "alter table t1 add constraint pk primary key (k1)"); addCommand.run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches(logicalOlapScan().when(o -> { - Constraint c = o.getTable().getConstraint("pk"); + TableNameInfo tni = new TableNameInfo(o.getTable()); + Constraint c = getConstraintMgr().getConstraint(tni, "pk"); if (c instanceof PrimaryKeyConstraint) { Set columns = ((PrimaryKeyConstraint) c).getPrimaryKeyNames(); return columns.size() == 1 && columns.iterator().next().equals("k1"); @@ -90,7 +97,8 @@ void primaryKeyConstraintTest() throws Exception { "alter table t1 drop constraint pk"); dropCommand.run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); } @Test @@ -99,7 +107,8 @@ void uniqueConstraintTest() throws Exception { "alter table t1 add constraint un unique (k1)"); command.run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches(logicalOlapScan().when(o -> { - Constraint c = o.getTable().getConstraint("un"); + TableNameInfo tni = new TableNameInfo(o.getTable()); + Constraint c = getConstraintMgr().getConstraint(tni, "un"); if (c instanceof UniqueConstraint) { Set columns = ((UniqueConstraint) c).getUniqueColumnNames(); return columns.size() == 1 && columns.iterator().next().equals("k1"); @@ -111,7 +120,8 @@ void uniqueConstraintTest() throws Exception { "alter table t1 drop constraint un"); dropCommand.run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); } @Test @@ -121,8 +131,8 @@ void foreignKeyConstraintTest() throws Exception { try { command.run(connectContext, null); } catch (Exception e) { - Assertions.assertEquals("Foreign key constraint requires a primary key constraint [k1] in t2", - e.getMessage()); + Assertions.assertTrue(e.getMessage().contains( + "Foreign key constraint requires a primary key constraint [k1] in")); } ((AddConstraintCommand) new NereidsParser().parseSingle( "alter table t2 add constraint pk primary key (k1, k2)")).run(connectContext, null); @@ -131,24 +141,29 @@ void foreignKeyConstraintTest() throws Exception { null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches(logicalOlapScan().when(o -> { - Constraint c = o.getTable().getConstraint("fk"); + TableNameInfo tni = new TableNameInfo(o.getTable()); + Constraint c = getConstraintMgr().getConstraint(tni, "fk"); if (c instanceof ForeignKeyConstraint) { ForeignKeyConstraint f = (ForeignKeyConstraint) c; - Column ref1 = f.getReferencedColumn(((SlotReference) o.getOutput().get(0)).getOriginalColumn().get().getName()); - Column ref2 = f.getReferencedColumn(((SlotReference) o.getOutput().get(1)).getOriginalColumn().get().getName()); + Column ref1 = f.getReferencedColumn(((SlotReference) o.getOutput().get(0)) + .getOriginalColumn().get().getName()); + Column ref2 = f.getReferencedColumn(((SlotReference) o.getOutput().get(1)) + .getOriginalColumn().get().getName()); return ref1.getName().equals("k1") && ref2.getName().equals("k2"); } return false; })); PlanChecker.from(connectContext).parse("select * from t2").analyze().matches(logicalOlapScan().when(o -> { - Constraint c = o.getTable().getConstraint("pk"); + TableNameInfo tni = new TableNameInfo(o.getTable()); + Constraint c = getConstraintMgr().getConstraint(tni, "pk"); if (c instanceof PrimaryKeyConstraint) { Set columnNames = ((PrimaryKeyConstraint) c).getPrimaryKeyNames(); - List foreignTables = ((PrimaryKeyConstraint) c).getForeignTables(); + java.util.List foreignTableInfos + = ((PrimaryKeyConstraint) c).getForeignTableInfos(); return columnNames.size() == 2 && columnNames.equals(Sets.newHashSet("k1", "k2")) - && foreignTables.size() == 1 && foreignTables.get(0).getName().equals("t1"); + && foreignTableInfos.size() == 1; } return false; })); @@ -158,7 +173,8 @@ void foreignKeyConstraintTest() throws Exception { "alter table t1 drop constraint fk"); dropCommand.run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); // drop pk and fk referenced it also should be dropped ((AddConstraintCommand) new NereidsParser().parseSingle( "alter table t1 add constraint fk foreign key (k1, k2) references t2(k1, k2)")).run(connectContext, @@ -167,9 +183,11 @@ void foreignKeyConstraintTest() throws Exception { .run(connectContext, null); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); PlanChecker.from(connectContext).parse("select * from t2").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); } @Test @@ -179,7 +197,8 @@ void cascadeDropTest() throws Exception { dropConstraint("alter table t1 drop constraint pk"); PlanChecker.from(connectContext).parse("select * from t2").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); addConstraint("alter table t1 add constraint pk primary key (k1)"); addConstraint("alter table t1 add constraint fk foreign key (k1) references t1(k1)"); @@ -187,10 +206,209 @@ void cascadeDropTest() throws Exception { addConstraint("alter table t3 add constraint fk foreign key (k1) references t1(k1)"); dropConstraint("alter table t1 drop constraint pk"); PlanChecker.from(connectContext).parse("select * from t1").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); PlanChecker.from(connectContext).parse("select * from t2").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); PlanChecker.from(connectContext).parse("select * from t3").analyze().matches( - logicalOlapScan().when(o -> o.getTable().getConstraintsMapUnsafe().isEmpty())); + logicalOlapScan().when(o -> getConstraintMgr() + .getConstraints(new TableNameInfo(o.getTable())).isEmpty())); + } + + @Test + void dropTableBlockedByForeignKeyTest() throws Exception { + // Setup: PK on t1, FK on t2 referencing t1 + addConstraint("alter table t1 add constraint pk_dt primary key (k1)"); + addConstraint("alter table t2 add constraint fk_dt foreign key (k1) references t1(k1)"); + + // Drop t1 should fail because t2's FK references t1's PK + Assertions.assertThrows(Exception.class, () -> { + executeSql("drop table t1"); + }); + + // Verify t1 still exists and constraints are intact + TableNameInfo t1Info = new TableNameInfo("internal", "test", "t1"); + Assertions.assertNotNull(getConstraintMgr().getConstraint(t1Info, "pk_dt")); + + // Cleanup: drop FK first, then PK + dropConstraint("alter table t2 drop constraint fk_dt"); + dropConstraint("alter table t1 drop constraint pk_dt"); + } + + @Test + void forceDropTableCascadesForeignKeyTest() throws Exception { + // Create new tables for this test to avoid affecting other tests + createTable("create table t_pk (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + createTable("create table t_fk (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + + addConstraint("alter table t_pk add constraint pk_force primary key (k1)"); + addConstraint("alter table t_fk add constraint fk_force foreign key (k1) references t_pk(k1)"); + + // Force drop t_pk should succeed and cascade-drop FK on t_fk + executeSql("drop table t_pk force"); + + // Verify FK on t_fk was cascade-dropped + TableNameInfo tFkInfo = new TableNameInfo("internal", "test", "t_fk"); + Assertions.assertTrue(getConstraintMgr().getConstraints(tFkInfo).isEmpty()); + + // Cleanup + executeSql("drop table t_fk force"); + } + + @Test + void dropColumnBlockedByConstraintTest() throws Exception { + // Create a table with non-key columns + createTable("create table t_schema (\n" + + " k1 int,\n" + + " v1 int,\n" + + " v2 int\n" + + ")\n" + + "unique key(k1)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + + // Add UNIQUE constraint on v1 + addConstraint("alter table t_schema add constraint un_v1 unique (v1)"); + + // Try to drop column v1 -> should fail because of constraint + Assertions.assertThrows(Exception.class, () -> { + executeSql("alter table t_schema drop column v1"); + }); + + // Drop constraint first, then drop column should not throw during validation + dropConstraint("alter table t_schema drop constraint un_v1"); + + // Cleanup + executeSql("drop table t_schema force"); + } + + @Test + void replaceTableWithConstraintsTest() throws Exception { + // Create tables for replace test + createTable("create table t_orig (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + createTable("create table t_repl (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + createTable("create table t_ref (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + + // Add PK on t_orig, FK on t_ref referencing t_orig + addConstraint("alter table t_orig add constraint pk_orig primary key (k1)"); + addConstraint("alter table t_ref add constraint fk_ref foreign key (k1) references t_orig(k1)"); + + // Replace t_orig with t_repl (no swap, t_orig gets dropped) -> should fail + // because t_orig's PK is referenced by t_ref's FK + Assertions.assertThrows(Exception.class, () -> { + executeSql("alter table t_orig replace with table t_repl " + + "properties(\"swap\"=\"false\")"); + }); + + // Drop FK first + dropConstraint("alter table t_ref drop constraint fk_ref"); + + // Now replace should succeed + executeSql("alter table t_orig replace with table t_repl " + + "properties(\"swap\"=\"false\")"); + + // After replace: t_repl is renamed to t_orig, old t_orig is dropped + TableNameInfo tOrigInfo = new TableNameInfo("internal", "test", "t_orig"); + Assertions.assertTrue(getConstraintMgr().getConstraints(tOrigInfo).isEmpty()); + + // Cleanup + executeSql("drop table if exists t_orig force"); + executeSql("drop table if exists t_repl force"); + executeSql("drop table if exists t_ref force"); + } + + @Test + void dropConstraintOnNonExistentTableTest() throws Exception { + // Simulate an external table scenario: a constraint exists in the manager + // but the table has been deleted by another system. + ConstraintManager mgr = getConstraintMgr(); + TableNameInfo ghostTable = new TableNameInfo("internal", "test", "ghost_table"); + PrimaryKeyConstraint pk = new PrimaryKeyConstraint("ghost_pk", Sets.newHashSet("col1")); + // Add via replay path to bypass table validation + mgr.addConstraint(ghostTable, "ghost_pk", pk, true); + Assertions.assertNotNull(mgr.getConstraint(ghostTable, "ghost_pk")); + + // Drop constraint via SQL — the table does not exist, but the command should still succeed + DropConstraintCommand dropCmd = (DropConstraintCommand) new NereidsParser().parseSingle( + "alter table test.ghost_table drop constraint ghost_pk"); + dropCmd.run(connectContext, null); + + // Constraint should be removed + Assertions.assertNull(mgr.getConstraint(ghostTable, "ghost_pk")); + } + + @Test + void renameTableUpdatesConstraintsTest() throws Exception { + // Create dedicated tables for rename test + createTable("create table t_rename_src (\n" + + " k1 int,\n" + + " k2 int\n" + + ")\n" + + "unique key(k1, k2)\n" + + "distributed by hash(k1) buckets 4\n" + + "properties(\n" + + " \"replication_num\"=\"1\"\n" + + ")"); + + addConstraint("alter table t_rename_src add constraint pk_rename primary key (k1)"); + TableNameInfo oldInfo = new TableNameInfo("internal", "test", "t_rename_src"); + Assertions.assertNotNull(getConstraintMgr().getConstraint(oldInfo, "pk_rename")); + + // Rename the table + executeSql("alter table t_rename_src rename t_rename_dst"); + + // Constraint should be accessible under the new name + TableNameInfo newInfo = new TableNameInfo("internal", "test", "t_rename_dst"); + Assertions.assertNotNull(getConstraintMgr().getConstraint(newInfo, "pk_rename")); + + // Old name should no longer have constraints + Assertions.assertTrue(getConstraintMgr().getConstraints(oldInfo).isEmpty()); + + // Cleanup + dropConstraint("alter table t_rename_dst drop constraint pk_rename"); + executeSql("drop table t_rename_dst force"); } } diff --git a/regression-test/suites/nereids_p0/pkfk/test_pk_fk_drop_table.groovy b/regression-test/suites/nereids_p0/pkfk/test_pk_fk_drop_table.groovy index bed59d7b8dc29e..cc1b6c05f10795 100644 --- a/regression-test/suites/nereids_p0/pkfk/test_pk_fk_drop_table.groovy +++ b/regression-test/suites/nereids_p0/pkfk/test_pk_fk_drop_table.groovy @@ -52,8 +52,16 @@ suite("test_pk_fk_drop_table") { ); alter table customer_test add constraint c_pk primary key(c_customer_sk); alter table store_sales_test add constraint ss_c_fk foreign key(ss_customer_sk) references customer_test(c_customer_sk); - drop table customer_test; """ + + test { + sql """ + drop table customer_test; + """ + + exception "primary key is referenced by foreign key" + } + // expect: not throw sql "show constraints from store_sales_test;" -} \ No newline at end of file +} From 97cfda73c67b87e7c2733c3b727b697c4729816d Mon Sep 17 00:00:00 2001 From: morrySnow Date: Wed, 11 Mar 2026 18:33:44 +0800 Subject: [PATCH 2/2] Fix addForeignTable dedup and DROP DATABASE FK ordering Bug 1: PrimaryKeyConstraint.addForeignTable() now deduplicates via foreignTableNameStrs (HashSet) before appending to foreignTableInfos (ArrayList). Prevents duplicate entries from rebuildForeignKeyReferences or duplicate editlog replay. Bug 2: Add ConstraintManager.dropDatabaseConstraints() to pre-clear all constraints for a database before iterating tables in unprotectDropDb(). This avoids non-deterministic FK check failures when table B has an FK referencing table A's PK and B happens to be iterated before A. Also refactored dropCatalogConstraints to share dropConstraintsByPrefix helper with the new dropDatabaseConstraints method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../org/apache/doris/catalog/TableIf.java | 1 - .../catalog/constraint/ConstraintManager.java | 51 +++++++++++++----- .../constraint/PrimaryKeyConstraint.java | 6 ++- .../doris/datasource/InternalCatalog.java | 5 ++ .../plans/commands/DropConstraintCommand.java | 1 + .../constraint/ConstraintManagerTest.java | 52 +++++++++++++++++++ 6 files changed, 100 insertions(+), 16 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java index 03a6d62b97aed3..f3cd4cbeb23bd8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java @@ -25,7 +25,6 @@ import org.apache.doris.datasource.systable.SysTable; import org.apache.doris.datasource.systable.TvfSysTable; import org.apache.doris.info.TableValuedFunctionRefInfo; -import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.functions.table.TableValuedFunction; import org.apache.doris.statistics.AnalysisInfo; import org.apache.doris.statistics.BaseAnalysisTask; diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java index 78745f3eaf61c2..7138dfd22a72a5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/ConstraintManager.java @@ -293,25 +293,50 @@ public void dropCatalogConstraints(String catalogName) { writeLock(); try { String prefix = catalogName + "."; - List tablesToRemove = constraintsMap.keySet().stream() - .filter(k -> k.startsWith(prefix)) - .collect(Collectors.toList()); - for (String tableName : tablesToRemove) { - Map tableConstraints - = constraintsMap.remove(tableName); - if (tableConstraints != null) { - for (Constraint constraint : tableConstraints.values()) { - cleanupConstraintReferencesOutsideCatalog( - tableName, constraint, prefix); - } - } - } + dropConstraintsByPrefix(prefix); LOG.info("Dropped all constraints for catalog {}", catalogName); } finally { writeUnlock(); } } + /** + * Remove all constraints for tables in the given database. + * Called during DROP DATABASE to pre-clear all intra-database FK references + * before individual table drops, avoiding ordering-dependent FK check failures. + */ + public void dropDatabaseConstraints(String catalogName, String dbName) { + writeLock(); + try { + String prefix = catalogName + "." + dbName + "."; + dropConstraintsByPrefix(prefix); + LOG.info("Dropped all constraints for database {}.{}", + catalogName, dbName); + } finally { + writeUnlock(); + } + } + + /** + * Remove all constraints whose qualified table name starts with + * the given prefix, cleaning up cross-references outside the prefix. + */ + private void dropConstraintsByPrefix(String prefix) { + List tablesToRemove = constraintsMap.keySet().stream() + .filter(k -> k.startsWith(prefix)) + .collect(Collectors.toList()); + for (String tableName : tablesToRemove) { + Map tableConstraints + = constraintsMap.remove(tableName); + if (tableConstraints != null) { + for (Constraint constraint : tableConstraints.values()) { + cleanupConstraintReferencesOutsideCatalog( + tableName, constraint, prefix); + } + } + } + } + /** * Move constraints from oldTableInfo to newTableInfo * and update all FK/PK references. Called when a table is renamed. diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java index 0876f7b425fe68..7e92a8627dca6d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/constraint/PrimaryKeyConstraint.java @@ -67,8 +67,10 @@ public Set getPrimaryKeys(TableIf table) { } public void addForeignTable(TableNameInfo tni) { - foreignTableInfos.add(tni); - foreignTableNameStrs.add(tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl()); + String key = tni.getCtl() + "." + tni.getDb() + "." + tni.getTbl(); + if (foreignTableNameStrs.add(key)) { + foreignTableInfos.add(tni); + } } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index cf0d790354d291..eef8b1a014ec18 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -563,6 +563,11 @@ public void dropDb(String dbName, boolean ifExists, boolean force) throws DdlExc public void unprotectDropDb(Database db, boolean isForeDrop, boolean isReplay, long recycleTime) throws DdlException { + // Pre-drop all constraints for this database to avoid ordering-dependent FK check failures. + // Without this, if table B has an FK referencing table A's PK, and B is iterated before A, + // dropping A would fail because B's FK still exists. + Env.getCurrentEnv().getConstraintManager().dropDatabaseConstraints( + InternalCatalog.INTERNAL_CATALOG_NAME, db.getFullName()); for (Table table : db.getTables()) { unprotectDropTable(db, table, isForeDrop, isReplay, recycleTime); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java index b4922b7da2182d..671cbd4ff9a358 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java @@ -36,6 +36,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.List; import java.util.Set; /** diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java index 456557c87d57c8..d629cc3ed4cf3e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintManagerTest.java @@ -371,6 +371,42 @@ void dropCatalogConstraintsOnNonExistentCatalogIsNoop() { Assertions.assertNotNull(mgr.getConstraint(T1, "pk")); } + // ==================== dropDatabaseConstraints ==================== + + @Test + void dropDatabaseConstraintsRemovesAllInDatabase() { + mgr.addConstraint(T1, "pk1", newPk("pk1", "k1"), true); + mgr.addConstraint(T2, "pk2", newPk("pk2", "k1"), true); + // T3 is in same db + mgr.addConstraint(T3, "uk3", new UniqueConstraint("uk3", ImmutableSet.of("c1")), true); + // Table in different database + TableNameInfo otherDbTable = new TableNameInfo("ctl", "other_db", "t1"); + mgr.addConstraint(otherDbTable, "pk_other", newPk("pk_other", "k1"), true); + + mgr.dropDatabaseConstraints("ctl", "db"); + + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + Assertions.assertTrue(mgr.getConstraints(T2).isEmpty()); + Assertions.assertTrue(mgr.getConstraints(T3).isEmpty()); + // Other database unaffected + Assertions.assertNotNull(mgr.getConstraint(otherDbTable, "pk_other")); + } + + @Test + void dropDatabaseConstraintsCascadesFKsAcrossDatabase() { + // PK in db, FK in other_db referencing the PK + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + TableNameInfo otherDbTable = new TableNameInfo("ctl", "other_db", "t1"); + mgr.addConstraint(otherDbTable, "fk", newFk("fk", T1, "c1", "k1"), true); + + mgr.dropDatabaseConstraints("ctl", "db"); + + Assertions.assertTrue(mgr.getConstraints(T1).isEmpty()); + // FK in other_db should be cascade-dropped because the referenced PK was removed + Assertions.assertTrue(mgr.getConstraints(otherDbTable).isEmpty(), + "FK in other_db should be cascade-dropped when referenced PK's database is dropped"); + } + // ==================== renameTable ==================== @Test @@ -536,6 +572,22 @@ void rebuildForeignKeyReferencesWiresFKToPK() { Assertions.assertFalse(loadedPk.getForeignTableInfos().isEmpty()); } + @Test + void rebuildForeignKeyReferencesDoesNotDuplicateEntries() { + // PK on T1, FK on T2 referencing T1 — registered via addConstraint + mgr.addConstraint(T1, "pk", newPk("pk", "k1"), true); + mgr.addConstraint(T2, "fk", newFk("fk", T1, "c1", "k1"), true); + + // addConstraint already registered T2 in PK's foreignTableInfos + PrimaryKeyConstraint pk = (PrimaryKeyConstraint) mgr.getConstraint(T1, "pk"); + Assertions.assertEquals(1, pk.getForeignTableInfos().size()); + + // rebuild should NOT add duplicates + mgr.rebuildForeignKeyReferences(); + Assertions.assertEquals(1, pk.getForeignTableInfos().size(), + "rebuildForeignKeyReferences should not duplicate entries"); + } + // ==================== Serialization ==================== @Test