Merge pull request #7158 from santysisi/fix/sqlite-fk-alter-table

Fix SQLite FK handling in altered tables

Author: morozov

UPGRADE.md

@@ -8,6 +8,11 @@ awareness about deprecated code.
 
 # Upgrade to 5.0
 
+## BC BREAK: changes in `Table` class
+
+The `Table` class no longer accepts `foreignKeyConstraints`
+with duplicate names.
+
 ## BC BREAK: changes in `TableDiff` constructor
 
 The `TableDiff` class constructor no longer accepts `droppedForeignKeys`

src/Platforms/SQLitePlatform.php

@@ -27,6 +27,7 @@
 use Doctrine\DBAL\Types;
 
 use function array_merge;
+use function assert;
 use function count;
 use function explode;
 use function implode;
@@ -862,6 +863,7 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
         $oldTable    = $diff->getOldTable();
         $foreignKeys = $oldTable->getForeignKeys();
         $nameMap     = $this->getDiffColumnNameMap($diff);
+        $keysByName  = [];
 
         foreach ($foreignKeys as $key => $constraint) {
             $changed = false;
@@ -884,6 +886,17 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                 $changed = true;
             }
 
+            $constraintName = $constraint->getObjectName();
+
+            if ($constraintName !== null) {
+                $constraintKey = strtolower(
+                    $constraintName->getIdentifier()
+                        ->getValue(),
+                );
+
+                $keysByName[$constraintKey] = $key;
+            }
+
             if (! $changed) {
                 continue;
             }
@@ -905,7 +918,8 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                     ->getValue(),
             );
 
-            unset($foreignKeys[$constraintKey]);
+            assert(isset($keysByName[$constraintKey]));
+            unset($foreignKeys[$keysByName[$constraintKey]], $keysByName[$constraintKey]);
         }
 
         foreach ($diff->getAddedForeignKeys() as $constraint) {
@@ -917,7 +931,10 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array
                         ->getValue(),
                 );
 
-                $foreignKeys[$constraintKey] = $constraint;
+                assert(! isset($keysByName[$constraintKey]));
+                $foreignKeys[] = $constraint;
+
+                $keysByName[$constraintKey] = count($foreignKeys) - 1;
             } else {
                 $foreignKeys[] = $constraint;
             }

src/Schema/Exception/ForeignKeyAlreadyExists.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Schema\Exception;
+
+use Doctrine\DBAL\Schema\Name\OptionallyQualifiedName;
+use Doctrine\DBAL\Schema\Name\UnqualifiedName;
+use Doctrine\DBAL\Schema\SchemaException;
+use LogicException;
+
+use function sprintf;
+
+final class ForeignKeyAlreadyExists extends LogicException implements SchemaException
+{
+    public static function new(OptionallyQualifiedName $tableName, UnqualifiedName $foreignKeyName): self
+    {
+        return new self(
+            sprintf(
+                'A foreign key constraint named %s already exists on table %s.',
+                $foreignKeyName->toString(),
+                $tableName->toString(),
+            ),
+        );
+    }
+}

src/Schema/Table.php

@@ -6,6 +6,7 @@
 
 use Doctrine\DBAL\Schema\Exception\ColumnAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\ColumnDoesNotExist;
+use Doctrine\DBAL\Schema\Exception\ForeignKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\ForeignKeyDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\IndexAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
@@ -839,12 +840,7 @@ private function _addForeignKeyConstraint(ForeignKeyConstraint $constraint): sel
         }
 
         if (isset($this->foreignKeyConstraints[$key])) {
-            Deprecation::trigger(
-                'doctrine/dbal',
-                'https://github.com/doctrine/dbal/pull/7125',
-                'Overwriting an existing foreign key constraint ("%s") is deprecated.',
-                $key,
-            );
+            throw ForeignKeyAlreadyExists::new($this->name, UnqualifiedName::unquoted($key));
         }
 
         $this->foreignKeyConstraints[$key] = $constraint;

tests/Schema/TableTest.php

@@ -8,6 +8,7 @@
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Schema\Column;
+use Doctrine\DBAL\Schema\Exception\ForeignKeyAlreadyExists;
 use Doctrine\DBAL\Schema\Exception\IndexDoesNotExist;
 use Doctrine\DBAL\Schema\Exception\InvalidForeignKeyConstraintDefinition;
 use Doctrine\DBAL\Schema\Exception\InvalidIndexDefinition;
@@ -1797,7 +1798,7 @@ public function testOverwritingForeignKeyConstraint(): void
 
         $table->addForeignKeyConstraint('bar', ['id'], ['id']);
 
-        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/7125');
+        $this->expectException(ForeignKeyAlreadyExists::class);
         $table->addForeignKeyConstraint('baz', ['id'], ['id']);
     }
 }