-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Foreign key name inconsistency may break migrations #6518
Comments
I had to drop all FKs before the breaking version // Version20240814000000.php
public function up(Schema $schema): void {
$sql = "SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.TABLE_CONSTRAINTS where table_schema = '$db' and constraint_type = 'FOREIGN KEY';";
$rows = $this->connection->iterateNumeric($sql);
foreach ($rows as [$table, $constraint]) {
$this->connection->executeStatement("ALTER TABLE $table DROP CONSTRAINT $constraint");
}
} Remove all FK statements from the breaking version Re-add all FKs with their proper names after the breaking version And added a postUp check to detect FK changes manually public function postUp(Schema $schema): void {
if ($this->debug && $this->version > 'Version20240814000000') {
$db = $this->connection->getDatabase();
$sql = "SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.TABLE_CONSTRAINTS where table_schema = '$db' and constraint_type = 'FOREIGN KEY';";
$rows = $this->connection->iterateNumeric($sql);
$actualFks = [];
foreach ($rows as [$table, $constraint]) {
$actualFks[$table][] = $constraint;
}
$tableReflection = new \ReflectionClass(Table::class);
$getMax = $tableReflection->getMethod('_getMaxIdentifierLength');
$fkReflection = new \ReflectionClass(ForeignKeyConstraint::class);
$getIdentifier = $fkReflection->getMethod('_generateIdentifierName');
$mismatch = false;
foreach ($schema->getTables() as $table) {
$max = $getMax->invoke($table);
$wanted = [];
foreach ($table->getForeignKeys() as $fk) {
$name = $getIdentifier->invoke($fk, array_merge([$table->getName()], $fk->getLocalColumns()), 'fk', $max);
$wanted[] = strtoupper($name);
}
sort($wanted);
$actual = $actualFks[$table->getName()] ?? [];
sort($actual);
$missing = array_diff($wanted, $actual);
$superfluous = array_diff($actual, $wanted);
if ($missing || $superfluous) {
$missing = implode(', ', $missing);
$superfluous = implode(', ', $superfluous);
$this->logger->error("Table {$table->getName()} has mismatching FK names! missing: [$missing], superfluous: [$superfluous]");
$mismatch = true;
}
}
if ($mismatch) {
throw new \RuntimeException('FK mismatch');
}
}
} While debugging I saw that 75 FKs in total had gone out of sync 🥹 |
#6520 is a pain to merge into 4.1... it's totally unclear to me, how to get the configuration into the comparator using the platform in between now. |
I'm targeting 4.2 now directly: #6521 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Report
Summary
Current behaviour
doctrine:migrations:diff
does not rename FKs if they are out of sync with the naming strategyHow to reproduce
doctrine:schema:create
DROP FOREIGN KEY
sExpected behaviour
This problem was solved with #6390, but then reverted again due to #6437.
I understand the problems in #6437, but legacy FKs shouldn't be the reason to prevent the migration system from working properly. I'm ok with having to opt-in into this, although I guess it would make more sense to offer a way to exclude certain custom/legacy FKs from the diff.
Right now I'm stuck with production servers not being able to update. And an update-path might be tricky. I might have to drop all FKs and re-add them with known names.
The text was updated successfully, but these errors were encountered: