Merge pull request #21 from camcima/hotfix/table-drops

DROP first

src/Differ.php

@@ -38,7 +38,10 @@ public function diffDatabases(Database $fromDatabase, Database $toDatabase, arra
             if ($fromTable->generateCreationScript(true) !== $toTable->generateCreationScript(true)) {
                 $changedTable = new ChangedTable($fromTable, $toTable);
                 $this->diffChangedTable($changedTable);
-                $databaseDiff->addChangedTable($changedTable);
+
+                if (!empty($changedTable->generateAlterScript())) {
+                    $databaseDiff->addChangedTable($changedTable);
+                }
             }
         }
 

src/Model/ChangedTable.php

@@ -303,30 +303,31 @@ public function addChangedForeignKey(ForeignKey $changedForeignKey)
      */
     public function generateAlterScript()
     {
+        $tableDrops = [];
         $tableChanges = [];
 
         if ($this->deletedPrimaryKey || (!empty($this->fromTable->getPrimaryKeys()) && !empty($this->changedPrimaryKeys))) {
-            $tableChanges[] = 'DROP PRIMARY KEY';
+            $tableDrops[] = 'DROP PRIMARY KEY';
         }
 
         foreach ($this->deletedForeignKeys as $deletedForeignKey) {
-            $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
+            $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $deletedForeignKey->getName());
         }
 
         foreach ($this->changedForeignKeys as $changedForeignKey) {
-            $tableChanges[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
+            $tableDrops[] = sprintf('DROP FOREIGN KEY `%s`', $changedForeignKey->getName());
         }
 
         foreach ($this->deletedIndexes as $deletedIndex) {
-            $tableChanges[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
+            $tableDrops[] = sprintf('DROP INDEX `%s`', $deletedIndex->getName());
         }
 
         foreach ($this->changedIndexes as $changedIndex) {
-            $tableChanges[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
+            $tableDrops[] = sprintf('DROP INDEX `%s`', $changedIndex->getName());
         }
 
         foreach ($this->deletedColumns as $deletedColumn) {
-            $tableChanges[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
+            $tableDrops[] = sprintf('DROP COLUMN `%s`', $deletedColumn->getName());
         }
 
         $columnStatements = [];
@@ -390,9 +391,17 @@ public function generateAlterScript()
             $tableChanges[] = sprintf('COMMENT=\'%s\'', str_replace('\'', '\'\'', $this->toTable->getComment()));
         }
 
-        $alterScript = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableChanges));
+        $alterScripts = [];
 
-        return $alterScript;
+        if (!empty($tableDrops)) {
+            $alterScripts[] = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableDrops));
+        }
+
+        if (!empty($tableChanges)) {
+            $alterScripts[] = sprintf('ALTER TABLE `%s`%s  %s;', $this->getName(), PHP_EOL, implode(',' . PHP_EOL . '  ', $tableChanges));
+        }
+
+        return implode(PHP_EOL, $alterScripts);
     }
 
     /**

src/RegExpPattern.php

@@ -35,7 +35,7 @@ class RegExpPattern
     public static function tables()
     {
         $pattern = '/(?<creationScript>CREATE\s+TABLE\s+(?<ifNotExists>IF NOT EXISTS)?\s*`(?<tableName>\S+)`\s+';
-        $pattern .= '\((?<tableDefinition>[^\/]+)\)';
+        $pattern .= '\((?<tableDefinition>[^\/]+?)\)';
         $pattern .= '(';
         $pattern .= '(?:\s+ENGINE\s*=\s*(?<engine>[^;\s]+))?\s*';
         $pattern .= '|';

tests/DifferTest.php

@@ -101,4 +101,17 @@ public function testIsGeneratingMigrationScript()
 
         $this->assertEquals($this->getDatabaseFixture('sakila_migration.sql'), $result);
     }
+
+    public function testIsDiffingDeletedForeignKeyColumn()
+    {
+        $parser = new Parser();
+
+        $fromDatabase = $parser->parseDatabase($this->getDatabaseFixture('fk_deleted_column1.sql'));
+        $toDatabase = $parser->parseDatabase($this->getDatabaseFixture('fk_deleted_column2.sql'));
+
+        $differ = new Differ();
+        $databaseDiff = $differ->diffDatabases($fromDatabase, $toDatabase);
+
+        $this->assertEquals($this->getDatabaseFixture('fk_deleted_column_migration.sql'), $differ->generateMigrationScript($databaseDiff));
+    }
 }

tests/fixtures/fk_deleted_column1.sql

@@ -0,0 +1,33 @@
+-- --------------------------------------------------------
+-- Host:                         127.0.0.1
+-- Server version:               5.7.10-log - MySQL Community Server (GPL)
+-- Server OS:                    Win64
+-- HeidiSQL Version:             9.4.0.5174
+-- --------------------------------------------------------
+
+/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
+/*!40101 SET NAMES utf8 */;
+/*!50503 SET NAMES utf8mb4 */;
+/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
+/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
+
+-- Dumping structure for table sakila.table_1
+CREATE TABLE IF NOT EXISTS `table_1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `foreignId` int(11) DEFAULT NULL,
+  PRIMARY KEY (`id`),
+  KEY `FK_table_1_table_2` (`foreignId`),
+  CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+-- Data exporting was unselected.
+-- Dumping structure for table sakila.table_2
+CREATE TABLE IF NOT EXISTS `table_2` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+-- Data exporting was unselected.
+/*!40101 SET SQL_MODE=IFNULL(@OLD_SQL_MODE, '') */;
+/*!40014 SET FOREIGN_KEY_CHECKS=IF(@OLD_FOREIGN_KEY_CHECKS IS NULL, 1, @OLD_FOREIGN_KEY_CHECKS) */;
+/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;

tests/fixtures/fk_deleted_column2.sql

@@ -0,0 +1,17 @@
+-- Host: 127.0.0.1    Database: test_2
+
+DROP TABLE IF EXISTS `table_1`;
+CREATE TABLE `table_1` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `foreignId2` int(11) DEFAULT NULL,
+  PRIMARY KEY (`id`),
+  KEY `FK_table_1_table_2` (`foreignId2`),
+  CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId2`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';
+
+
+DROP TABLE IF EXISTS `table_2`;
+CREATE TABLE `table_2` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='.';

tests/fixtures/fk_deleted_column_migration.sql

@@ -0,0 +1,23 @@
+# Disable Foreign Keys Check
+SET FOREIGN_KEY_CHECKS = 0;
+SET SQL_MODE = '';
+
+# Deleted Tables
+
+# Changed Tables
+
+-- changed table `table_1`
+
+ALTER TABLE `table_1`
+  DROP FOREIGN KEY `FK_table_1_table_2`,
+  DROP INDEX `FK_table_1_table_2`,
+  DROP COLUMN `foreignId`;
+ALTER TABLE `table_1`
+  ADD COLUMN `foreignId2` int(11) DEFAULT NULL AFTER `id`,
+  ADD KEY `FK_table_1_table_2` (`foreignId2`),
+  ADD CONSTRAINT `FK_table_1_table_2` FOREIGN KEY (`foreignId2`) REFERENCES `table_2` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION;
+
+# New Tables
+
+# Disable Foreign Keys Check
+SET FOREIGN_KEY_CHECKS = 1;

tests/fixtures/sakila_migration.sql

@@ -15,7 +15,8 @@ DROP TABLE `test1`;
 ALTER TABLE `test2`
   DROP PRIMARY KEY,
   DROP FOREIGN KEY `FK__test1`,
-  DROP INDEX `FK__test1`,
+  DROP INDEX `FK__test1`;
+ALTER TABLE `test2`
   CHANGE COLUMN `id` `id` int(10) NOT NULL AUTO_INCREMENT FIRST,
   CHANGE COLUMN `fk` `fk` int(10) AFTER `id`,
   CHANGE COLUMN `val` `val` decimal(11,3) NOT NULL AFTER `fk`,