Fix handling of unnamed foreign key drops in SQLite #7143
Conversation
|
I added the test to |
SQLite ignored unnamed foreign key drops during schema diffs, leading to duplicate constraints. This uses the diff key as a fallback name to ensure unnamed FKs are removed properly.
santysisi
force-pushed
the
fix/sqlite-drop-unnamed-foreign-keys
branch
from
54bae1a to
c2852f3
Compare
| @@ -929,8 +930,8 @@ private function getForeignKeysInAlteredTable(TableDiff $diff): array | |||
| ); | |||
| } | |||
|
|
|||
| foreach ($diff->getDroppedForeignKeys() as $constraint) { | |||
| $constraintName = $constraint->getName(); | |||
| foreach ($diff->getDroppedForeignKeys() as $name => $constraint) { | |||
There was a problem hiding this comment.
This is against the effort made in #7102. The constraint name is represented by the constraint object itself, not by some array key.
There was a problem hiding this comment.
Got it, thanks for pointing that out! I’ll look into applying a different approach that aligns better with the changes from #7102 as soon as possible.
There was a problem hiding this comment.
It might be a good idea to just deprecate the scenario when the constraint being dropped doesn't have a name. Currently, we're not doing anything, which causes a bug. Making it work correctly would require a deeper redesign of schema management.
Just for context, the fundamental flaw is the following. The test attempts to perform a schema migration in the following steps:
- Define two versions of a table.
- Calculate a difference between them.
- Apply the difference.
The problem is that:
- The calculation of the difference is non-deterministic (you cannot reliably tell whether a given column was renamed or dropped and created). This doesn't apply to this specific case, but still.
- SQLite doesn't support
ALTER TABLE. The DBAL has to recreate it from scratch and migrate the data.
The combination of these flaws leads to the fact that we cannot migrate schemas with unnamed constraints. A proper design for SQLite would not involve the diff at all. For SQLite, it would use the expected table schema directly.
Summary
This PR fixes an issue where unnamed foreign keys were not dropped in SQLite during schema diffs.