doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.93k stars 2.52k forks source link

DBAL schema update fails in safe mode on MySQL due to orphaned foreign keys #6609

Open mastir opened 7 years ago

mastir commented 7 years ago

finaly i've found this. "General Error 1451: Cannot delete or update a parent row: a foreign key constraint fails" in every second update using Doctrine SchemaTool.

The problem: we do remove some foreignKeys updates in TableDiff (not full = invalid).

            // deleting duplicated foreign keys present on both on the orphanedForeignKey
            // and the removedForeignKeys from changedTables
            foreach ($foreignKeysToTable[$tableName] as $foreignKey) {
                // strtolower the table name to make if compatible with getShortestName
                $localTableName = strtolower($foreignKey->getLocalTableName());
                if (isset($diff->changedTables[$localTableName])) {
                    foreach ($diff->changedTables[$localTableName]->removedForeignKeys as $key => $removedForeignKey) {
                        unset($diff->changedTables[$localTableName]->removedForeignKeys[$key]);
                    }
                }
            }

And right after it (cose of it) we got bugs. Here we skip orphanedForeignKeys in safeMode. And this keys we allready removed from table diff. So them gone forever, getDropForeignKeySQL is never used.

    if ($platform->supportsForeignKeyConstraints() && $saveMode == false) {
        foreach ($this->orphanedForeignKeys as $orphanedForeignKey) {
            $removedForeignKeys[] = $orphanedForeignKey;
            $sql[] = $platform->getDropForeignKeySQL($orphanedForeignKey, $orphanedForeignKey->getLocalTableName());
        }
    }

Simple solution is not to invalidate TableDiff and check for double drop foreignKeys.

diff --git a/actual/lib/vendor/Doctrine/DBAL/Schema/Comparator.php b/actual/lib/vendor/Doctrine/DBAL/Schema/Comparator.php
index 4f11e6c..88cead3 100644
--- a/actual/lib/vendor/Doctrine/DBAL/Schema/Comparator.php
+++ b/actual/lib/vendor/Doctrine/DBAL/Schema/Comparator.php
@@ -109,17 +109,6 @@ class Comparator
             if (isset($foreignKeysToTable[$tableName])) {
                 $diff->orphanedForeignKeys = array_merge($diff->orphanedForeignKeys, $foreignKeysToTable[$tableName]);

-                // deleting duplicated foreign keys present on both on the orphanedForeignKey
-                // and the removedForeignKeys from changedTables
-                foreach ($foreignKeysToTable[$tableName] as $foreignKey) {
-                    // strtolower the table name to make if compatible with getShortestName
-                    $localTableName = strtolower($foreignKey->getLocalTableName());
-                    if (isset($diff->changedTables[$localTableName])) {
-                        foreach ($diff->changedTables[$localTableName]->removedForeignKeys as $key => $removedForeignKey) {
-                            unset($diff->changedTables[$localTableName]->removedForeignKeys[$key]);
-                        }
-                    }
-                }
             }
         }

diff --git a/actual/lib/vendor/Doctrine/DBAL/Schema/SchemaDiff.php b/actual/lib/vendor/Doctrine/DBAL/Schema/SchemaDiff.php
index 35afe1f..80fbd69 100644
--- a/actual/lib/vendor/Doctrine/DBAL/Schema/SchemaDiff.php
+++ b/actual/lib/vendor/Doctrine/DBAL/Schema/SchemaDiff.php
@@ -145,6 +145,7 @@ class SchemaDiff
     protected function _toSql(AbstractPlatform $platform, $saveMode = false)
     {
         $sql = array();
+        $removedForeignKeys = [];

         if ($platform->supportsSchemas()) {
             foreach ($this->newNamespaces as $newNamespace) {
@@ -154,6 +155,7 @@ class SchemaDiff

         if ($platform->supportsForeignKeyConstraints() && $saveMode == false) {
             foreach ($this->orphanedForeignKeys as $orphanedForeignKey) {
+                $removedForeignKeys[] = $orphanedForeignKey;
                 $sql[] = $platform->getDropForeignKeySQL($orphanedForeignKey, $orphanedForeignKey->getLocalTableName());
             }
         }
@@ -196,7 +198,10 @@ class SchemaDiff
         }

         foreach ($this->changedTables as $tableDiff) {
+            $originKeys = $tableDiff->removedForeignKeys;
+            $tableDiff->removedForeignKeys = array_udiff($originKeys, $removedForeignKeys, function($a,$b){return ($a===$b)?0:1;});
             $sql = array_merge($sql, $platform->getAlterTableSQL($tableDiff));
+            $tableDiff->removedForeignKeys = $originKeys;
         }

         return $sql;

But it is not realy good solution, cose in context of AbstractPlatform we still got modified TableDiff (not full = invalid). And somewhere in AbstractPlatform declarations we can have a new bugs cose of it.

Ocramius commented 7 years ago

This needs a test before a solution.