doctrine/dbal

Foreign key name inconsistency may break migrations

Opened this issue · 3 comments

Bug Report

Q A
Version 4.1.0, but introduced earlier

Summary

  • Doctrine schema tools determine the names of foreign keys
  • At some point in the past they stopped tracking name changes (I guess somewhere in 3.x) in migration diffs
  • This leads to broken migrations when the FK names are different, but need to be dropped

Current behaviour

  • doctrine:migrations:diff does not rename FKs if they are out of sync with the naming strategy

How to reproduce

  • Database A was created some time ago and is used in development to create migration diffs
  • At some point table names or columns were renamed
  • Now the FK names are out of sync with what Doctrine would name them if they were freshly created
  • Now create database B with doctrine:schema:create
  • Database B will have the new FK names
  • We rename tables/columns, create a diff based on database A, which includes DROP FOREIGN KEYs
  • This diff is incompatible with database B because the named FKs don't exist there
  • The migration crashes on database B

Expected behaviour

  • If Doctrine names FKs itself, then it must track the names in all diffs itself, too.
  • There is no other way in keeping schemas in sync and not create a scenario where migrations fail.

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.

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