Merge pull request #7166 from morozov/remove-dropped-foreign-key-constraints

morozov · web-flow · commit 09ee3b177f72 · 2025-09-28T06:58:25.000-07:00
Remove TableDiff::getDroppedForeignKeys()
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -8,6 +8,12 @@ awareness about deprecated code.
 
 # Upgrade to 5.0
 
+## BC BREAK: Removed `TableDiff::getDroppedForeignKeys()`
+
+The `TableDiff::getDroppedForeignKeys()` method has been removed.
+
+An attempt to drop an unnamed foreign key constraint on SQLite will result in an exception.
+
 ## BC BREAK: Removed `AbstractSchemaManager` methods
 
 The following `AbstractSchemaManager` methods have been removed:
diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php
@@ -947,8 +947,14 @@ public function getDropTablesSQL(array $tables): array
 
         foreach ($tables as $table) {
             foreach ($table->getForeignKeys() as $foreignKey) {
+                $constraintName = $foreignKey->getObjectName();
+
+                if ($constraintName === null) {
+                    throw UnspecifiedConstraintName::forForeignKeyConstraint();
+                }
+
                 $sql[] = $this->getDropForeignKeySQL(
-                    $this->getConstraintName($foreignKey)->toSQL($this),
+                    $constraintName->toSQL($this),
                     $table->getObjectName()->toSQL($this),
                 );
             }
@@ -1220,8 +1226,8 @@ protected function getPreAlterTableIndexForeignKeySQL(TableDiff $diff): array
 
         $sql = [];
 
-        foreach ($diff->getDroppedForeignKeys() as $foreignKey) {
-            $sql[] = $this->getDropForeignKeySQL($this->getConstraintName($foreignKey)->toSQL($this), $tableNameSQL);
+        foreach ($diff->getDroppedForeignKeyConstraintNames() as $constraintName) {
+            $sql[] = $this->getDropForeignKeySQL($constraintName->toSQL($this), $tableNameSQL);
         }
 
         foreach ($diff->getDroppedIndexes() as $index) {
@@ -2210,15 +2216,4 @@ abstract public function createMetadataProvider(Connection $connection): Metadat
      * database schema according to the dialect of the platform.
      */
     abstract public function createSchemaManager(Connection $connection): AbstractSchemaManager;
-
-    private function getConstraintName(ForeignKeyConstraint $constraint): UnqualifiedName
-    {
-        $name = $constraint->getObjectName();
-
-        if ($name === null) {
-            throw UnspecifiedConstraintName::new();
-        }
-
-        return $name;
-    }
 }
diff --git a/src/Platforms/DB2Platform.php b/src/Platforms/DB2Platform.php
@@ -273,7 +273,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $constraintName = $droppedPrimaryKeyConstraint->getObjectName();
 
             if ($constraintName === null) {
-                throw UnspecifiedConstraintName::new();
+                throw UnspecifiedConstraintName::forPrimaryKeyConstraint();
             }
 
             $sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);
diff --git a/src/Platforms/OraclePlatform.php b/src/Platforms/OraclePlatform.php
@@ -510,7 +510,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $constraintName = $droppedPrimaryKeyConstraint->getObjectName();
 
             if ($constraintName === null) {
-                throw UnspecifiedConstraintName::new();
+                throw UnspecifiedConstraintName::forPrimaryKeyConstraint();
             }
 
             $sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);
diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php
@@ -185,7 +185,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $constraintName = $droppedPrimaryKeyConstraint->getObjectName();
 
             if ($constraintName === null) {
-                throw UnspecifiedConstraintName::new();
+                throw UnspecifiedConstraintName::forPrimaryKeyConstraint();
             }
 
             $sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);
diff --git a/src/Platforms/SQLServerPlatform.php b/src/Platforms/SQLServerPlatform.php
@@ -321,7 +321,7 @@ public function getAlterTableSQL(TableDiff $diff): array
             $constraintName = $droppedPrimaryKeyConstraint->getObjectName();
 
             if ($constraintName === null) {
-                throw UnspecifiedConstraintName::new();
+                throw UnspecifiedConstraintName::forPrimaryKeyConstraint();
             }
 
             $sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $table->getObjectName()->toSQL($this));
diff --git a/src/Platforms/SQLitePlatform.php b/src/Platforms/SQLitePlatform.php
@@ -719,7 +719,7 @@ private function getSimpleAlterTableSQL(TableDiff $diff): array|false
             || count($diff->getDroppedIndexes()) > 0
             || count($diff->getRenamedIndexes()) > 0
             || count($diff->getAddedForeignKeys()) > 0
-            || count($diff->getDroppedForeignKeys()) > 0
+            || count($diff->getDroppedForeignKeyConstraintNames()) > 0
             || $diff->getDroppedPrimaryKeyConstraint() !== null
             || $diff->getAddedPrimaryKeyConstraint() !== null
         ) {
@@ -901,13 +901,7 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                 ->create();
         }
 
-        foreach ($diff->getDroppedForeignKeys() as $constraint) {
-            $constraintName = $constraint->getObjectName();
-
-            if ($constraintName === null) {
-                continue;
-            }
-
+        foreach ($diff->getDroppedForeignKeyConstraintNames() as $constraintName) {
             $constraintKey = strtolower(
                 $constraintName->getIdentifier()
                     ->getValue(),
diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php
@@ -5,6 +5,7 @@
 namespace Doctrine\DBAL\Schema;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Schema\Exception\UnspecifiedConstraintName;
 use Doctrine\DBAL\Schema\Name\UnquotedIdentifierFolding;
 
 use function count;
@@ -127,16 +128,16 @@ public function diffSequence(Sequence $sequence1, Sequence $sequence2): bool
      */
     public function compareTables(Table $oldTable, Table $newTable): TableDiff
     {
-        $addedColumns                = [];
-        $modifiedColumns             = [];
-        $droppedColumns              = [];
-        $addedIndexes                = [];
-        $droppedIndexes              = [];
-        $renamedIndexes              = [];
-        $addedForeignKeys            = [];
-        $droppedForeignKeys          = [];
-        $addedPrimaryKeyConstraint   = null;
-        $droppedPrimaryKeyConstraint = null;
+        $addedColumns                     = [];
+        $modifiedColumns                  = [];
+        $droppedColumns                   = [];
+        $addedIndexes                     = [];
+        $droppedIndexes                   = [];
+        $renamedIndexes                   = [];
+        $addedForeignKeys                 = [];
+        $droppedForeignKeyConstraintNames = [];
+        $addedPrimaryKeyConstraint        = null;
+        $droppedPrimaryKeyConstraint      = null;
 
         $oldColumns = $oldTable->getColumns();
         $newColumns = $newTable->getColumns();
@@ -279,8 +280,8 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff
                         && strtolower($oldForeignKeyName->getIdentifier()->getValue())
                         === strtolower($newForeignKeyName->getIdentifier()->getValue())
                     ) {
-                        $droppedForeignKeys[$oldKey] = $oldForeignKey;
-                        $addedForeignKeys[$newKey]   = $newForeignKey;
+                        $droppedForeignKeyConstraintNames[$oldKey] = $oldForeignKeyName;
+                        $addedForeignKeys[$newKey]                 = $newForeignKey;
 
                         unset($oldForeignKeys[$oldKey], $newForeignKeys[$newKey]);
                     }
@@ -289,7 +290,12 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff
         }
 
         foreach ($oldForeignKeys as $oldForeignKey) {
-            $droppedForeignKeys[] = $oldForeignKey;
+            $constraintName = $oldForeignKey->getObjectName();
+            if ($constraintName === null) {
+                throw UnspecifiedConstraintName::forForeignKeyConstraint();
+            }
+
+            $droppedForeignKeyConstraintNames[] = $constraintName;
         }
 
         foreach ($newForeignKeys as $newForeignKey) {
@@ -305,7 +311,7 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff
             droppedIndexes: $droppedIndexes,
             renamedIndexes: $renamedIndexes,
             addedForeignKeys: $addedForeignKeys,
-            droppedForeignKeys: $droppedForeignKeys,
+            droppedForeignKeyConstraintNames: $droppedForeignKeyConstraintNames,
             addedPrimaryKeyConstraint: $addedPrimaryKeyConstraint,
             droppedPrimaryKeyConstraint: $droppedPrimaryKeyConstraint,
         );
diff --git a/src/Schema/Exception/UnspecifiedConstraintName.php b/src/Schema/Exception/UnspecifiedConstraintName.php
@@ -9,8 +9,13 @@
 
 final class UnspecifiedConstraintName extends InvalidArgumentException implements SchemaException
 {
-    public static function new(): self
+    public static function forPrimaryKeyConstraint(): self
     {
-        return new self('Constraint name is not specified.');
+        return new self('Primary key constraint name is not specified.');
+    }
+
+    public static function forForeignKeyConstraint(): self
+    {
+        return new self('Foreign key constraint name is not specified.');
     }
 }
diff --git a/src/Schema/SQLiteSchemaManager.php b/src/Schema/SQLiteSchemaManager.php
@@ -27,11 +27,17 @@ public function createForeignKey(ForeignKeyConstraint $foreignKey, string $table
 
     public function dropForeignKey(string $name, string $table): void
     {
-        $table = $this->introspectTableByStringName($table);
+        $parser = Parsers::getUnqualifiedNameParser();
+
+        try {
+            $parsedName = $parser->parse($name);
+        } catch (Parser\Exception $e) {
+            throw InvalidName::fromParserException($name, $e);
+        }
 
-        $foreignKey = $table->getForeignKey($name);
+        $table = $this->introspectTableByStringName($table);
 
-        $this->alterTable(new TableDiff($table, droppedForeignKeys: [$foreignKey]));
+        $this->alterTable(new TableDiff($table, droppedForeignKeyConstraintNames: [$parsedName]));
     }
 
     /** @throws Exception */
diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php
@@ -4,8 +4,6 @@
 
 namespace Doctrine\DBAL\Schema;
 
-use Doctrine\DBAL\Exception\InvalidArgumentException;
-use Doctrine\DBAL\Schema\Exception\InvalidState;
 use Doctrine\DBAL\Schema\Name\UnqualifiedName;
 use Doctrine\Deprecations\Deprecation;
 
@@ -23,14 +21,14 @@ final class TableDiff
      *
      * @internal The diff can be only instantiated by a {@see Comparator}.
      *
-     * @param array<ForeignKeyConstraint> $droppedForeignKeys
      * @param array<Column>               $addedColumns
      * @param array<string, ColumnDiff>   $changedColumns
      * @param array<Column>               $droppedColumns
      * @param array<Index>                $addedIndexes
      * @param array<Index>                $droppedIndexes
      * @param array<string, Index>        $renamedIndexes
      * @param array<ForeignKeyConstraint> $addedForeignKeys
+     * @param array<UnqualifiedName>      $droppedForeignKeyConstraintNames
      */
     public function __construct(
         private readonly Table $oldTable,
@@ -41,17 +39,10 @@ public function __construct(
         private array $droppedIndexes = [],
         private readonly array $renamedIndexes = [],
         private readonly array $addedForeignKeys = [],
-        private readonly array $droppedForeignKeys = [],
+        private readonly array $droppedForeignKeyConstraintNames = [],
         private readonly ?PrimaryKeyConstraint $addedPrimaryKeyConstraint = null,
         private readonly ?PrimaryKeyConstraint $droppedPrimaryKeyConstraint = null,
     ) {
-        foreach ($droppedForeignKeys as $droppedForeignKey) {
-            if ($droppedForeignKey->getObjectName() === null) {
-                throw new InvalidArgumentException(
-                    'Dropping a foreign key constraints without specifying its name is not allowed.',
-                );
-            }
-        }
     }
 
     public function getOldTable(): Table
@@ -179,31 +170,10 @@ public function getAddedForeignKeys(): array
         return $this->addedForeignKeys;
     }
 
-    /**
-     * @deprecated Use {@see getDroppedForeignKeyConstraintNames()}.
-     *
-     * @return array<ForeignKeyConstraint>
-     */
-    public function getDroppedForeignKeys(): array
-    {
-        return $this->droppedForeignKeys;
-    }
-
     /** @return array<UnqualifiedName> */
     public function getDroppedForeignKeyConstraintNames(): array
     {
-        $names = [];
-        foreach ($this->droppedForeignKeys as $constraint) {
-            $name = $constraint->getObjectName();
-
-            if ($name === null) {
-                throw InvalidState::tableDiffContainsUnnamedDroppedForeignKeyConstraints();
-            }
-
-            $names[] = $name;
-        }
-
-        return $names;
+        return $this->droppedForeignKeyConstraintNames;
     }
 
     public function getAddedPrimaryKeyConstraint(): ?PrimaryKeyConstraint
@@ -228,7 +198,7 @@ public function isEmpty(): bool
             && count($this->droppedIndexes) === 0
             && count($this->renamedIndexes) === 0
             && count($this->addedForeignKeys) === 0
-            && count($this->droppedForeignKeys) === 0
+            && count($this->droppedForeignKeyConstraintNames) === 0
             && $this->addedPrimaryKeyConstraint === null
             && $this->droppedPrimaryKeyConstraint === null;
     }
diff --git a/tests/Functional/Schema/ComparatorTest.php b/tests/Functional/Schema/ComparatorTest.php
@@ -6,11 +6,15 @@
 
 use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\MariaDBPlatform;
+use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\Comparator;
 use Doctrine\DBAL\Schema\ComparatorConfig;
+use Doctrine\DBAL\Schema\Exception\UnspecifiedConstraintName;
+use Doctrine\DBAL\Schema\ForeignKeyConstraint;
 use Doctrine\DBAL\Schema\Name\UnqualifiedName;
+use Doctrine\DBAL\Schema\PrimaryKeyConstraint;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Tests\Functional\Platform\RenameColumnTest;
 use Doctrine\DBAL\Tests\FunctionalTestCase;
@@ -151,4 +155,58 @@ public static function defaultValueProvider(): iterable
             [Types::TEXT, 'Doctrine'],
         ];
     }
+
+    public function testDropUnnamedForeignKeyConstraint(): void
+    {
+        $this->dropTableIfExists('tree');
+
+        $table1 = Table::editor()
+            ->setUnquotedName('tree')
+            ->setColumns(
+                Column::editor()
+                    ->setUnquotedName('id')
+                    ->setTypeName(Types::INTEGER)
+                    ->create(),
+                Column::editor()
+                    ->setUnquotedName('parent_id')
+                    ->setTypeName(Types::INTEGER)
+                    ->create(),
+            )
+            ->setPrimaryKeyConstraint(
+                PrimaryKeyConstraint::editor()
+                    ->setUnquotedColumnNames('id')
+                    ->create(),
+            )
+            ->setForeignKeyConstraints(
+                ForeignKeyConstraint::editor()
+                    ->setUnquotedReferencingColumnNames('parent_id')
+                    ->setUnquotedReferencedTableName('tree')
+                    ->setUnquotedReferencedColumnNames('id')
+                    ->create(),
+            )
+            ->create();
+
+        $this->schemaManager->createTable($table1);
+
+        $table1 = $this->schemaManager->introspectTableByUnquotedName('tree');
+
+        $table2 = $table1->edit()
+            ->setForeignKeyConstraints()
+            ->create();
+
+        $comparator = $this->schemaManager->createComparator();
+
+        // SQLite does not automatically generate names for unnamed constraints
+        if ($this->connection->getDatabasePlatform() instanceof SQLitePlatform) {
+            $this->expectException(UnspecifiedConstraintName::class);
+        }
+
+        $diff = $comparator->compareTables($table1, $table2);
+
+        $this->schemaManager->alterTable($diff);
+
+        $table2 = $this->schemaManager->introspectTableByUnquotedName('tree');
+
+        self::assertEmpty($table2->getForeignKeys());
+    }
 }
diff --git a/tests/Schema/AbstractComparatorTestCase.php b/tests/Schema/AbstractComparatorTestCase.php
diff --git a/tests/Schema/TableDiffTest.php b/tests/Schema/TableDiffTest.php

Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,7 @@ public function getAlterTableSQL(TableDiff $diff): array`
`273`	`273`	`$constraintName = $droppedPrimaryKeyConstraint->getObjectName();`
`274`	`274`
`275`	`275`	`if ($constraintName === null) {`
`276`		`- throw UnspecifiedConstraintName::new();`
	`276`	`+ throw UnspecifiedConstraintName::forPrimaryKeyConstraint();`
`277`	`277`	`}`
`278`	`278`
`279`	`279`	`$sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);`
Original file line number	Diff line number	Diff line change
`@@ -510,7 +510,7 @@ public function getAlterTableSQL(TableDiff $diff): array`
`510`	`510`	`$constraintName = $droppedPrimaryKeyConstraint->getObjectName();`
`511`	`511`
`512`	`512`	`if ($constraintName === null) {`
`513`		`- throw UnspecifiedConstraintName::new();`
	`513`	`+ throw UnspecifiedConstraintName::forPrimaryKeyConstraint();`
`514`	`514`	`}`
`515`	`515`
`516`	`516`	`$sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);`
Original file line number	Diff line number	Diff line change
`@@ -185,7 +185,7 @@ public function getAlterTableSQL(TableDiff $diff): array`
`185`	`185`	`$constraintName = $droppedPrimaryKeyConstraint->getObjectName();`
`186`	`186`
`187`	`187`	`if ($constraintName === null) {`
`188`		`- throw UnspecifiedConstraintName::new();`
	`188`	`+ throw UnspecifiedConstraintName::forPrimaryKeyConstraint();`
`189`	`189`	`}`
`190`	`190`
`191`	`191`	`$sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $tableNameSQL);`
Original file line number	Diff line number	Diff line change
`@@ -321,7 +321,7 @@ public function getAlterTableSQL(TableDiff $diff): array`
`321`	`321`	`$constraintName = $droppedPrimaryKeyConstraint->getObjectName();`
`322`	`322`
`323`	`323`	`if ($constraintName === null) {`
`324`		`- throw UnspecifiedConstraintName::new();`
	`324`	`+ throw UnspecifiedConstraintName::forPrimaryKeyConstraint();`
`325`	`325`	`}`
`326`	`326`
`327`	`327`	`$sql[] = $this->getDropConstraintSQL($constraintName->toSQL($this), $table->getObjectName()->toSQL($this));`
Original file line number	Diff line number	Diff line change
`@@ -9,8 +9,13 @@`
`9`	`9`
`10`	`10`	`final class UnspecifiedConstraintName extends InvalidArgumentException implements SchemaException`
`11`	`11`	`{`
`12`		`- public static function new(): self`
	`12`	`+ public static function forPrimaryKeyConstraint(): self`
`13`	`13`	`{`
`14`		`- return new self('Constraint name is not specified.');`
	`14`	`+ return new self('Primary key constraint name is not specified.');`
	`15`	`+ }`
	`16`	`+`
	`17`	`+ public static function forForeignKeyConstraint(): self`
	`18`	`+ {`
	`19`	`+ return new self('Foreign key constraint name is not specified.');`
`15`	`20`	`}`
`16`	`21`	`}`