Fix issue with dropColumn command in SQLite (#1086) (#1093)

* Fix issue with dropColumn command in SQLite (#1086) * Ensure fix for issue #1086 is compatible with PHP 5.4 * Rolled back PDO errormode setting and PDO testing fake constructor changes
cakephp · Jul 12, 2017 · dafa841 · dafa841
1 parent 789fe5d
commit dafa841
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 19 deletions.
diff --git a/src/Phinx/Db/Adapter/SQLiteAdapter.php b/src/Phinx/Db/Adapter/SQLiteAdapter.php
@@ -491,29 +491,40 @@ public function dropColumn($tableName, $columnName)
             ));
         }
 
+        // rename existing table to a temp table
         $this->execute(sprintf('ALTER TABLE %s RENAME TO %s', $tableName, $tmpTableName));
 
-        $sql = preg_replace(
-            sprintf("/%s\s%s[^,)]*(,\s|\))/", preg_quote($this->quoteColumnName($columnName)), preg_quote($columnType)),
-            "",
-            $sql
+        $originalBuildTableCommandIncludingColumnToRemove = $sql;
+
+        // isolate and rebuild column definitions sans the column we no longer want
+        $columnDefinitionsSubstring = preg_replace([
+                '/^CREATE\sTABLE\s`[^`]+`\s\(/i',
+                '/\);?\s*$/'
+            ],
+            '',
+            $originalBuildTableCommandIncludingColumnToRemove
         );
 
-        if (substr($sql, -2) === ', ') {
-            $sql = substr($sql, 0, -2) . ')';
-        }
+        $columnDefinitions = preg_split('/,\s*(?=`)/i', $columnDefinitionsSubstring);
+        $columnDefinitionsWithoutTheUnwantedColumn = array_filter($columnDefinitions, function ($definition) use ($columnName, $columnType) {
+            return (strpos($definition, $this->quoteColumnName($columnName).' '.$columnType) !== 0);
+        });
 
-        $this->execute($sql);
+        // rebuild the table creation command, without the unwanted column
+        $rebuildTableCommand = "CREATE TABLE `{$tableName}` (".implode(', ', $columnDefinitionsWithoutTheUnwantedColumn).')';
 
-        $sql = sprintf(
+        // rebuild the table
+        $this->execute($rebuildTableCommand);
+
+        $dataMigrationFromTempTableToNewTable = sprintf(
             'INSERT INTO %s(%s) SELECT %s FROM %s',
             $tableName,
             implode(', ', $columns),
             implode(', ', $columns),
             $tmpTableName
         );
 
-        $this->execute($sql);
+        $this->execute($dataMigrationFromTempTableToNewTable);
         $this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tmpTableName)));
         $this->endCommandTimer();
     }

diff --git a/tests/Phinx/Db/Adapter/SQLiteAdapterTest.php b/tests/Phinx/Db/Adapter/SQLiteAdapterTest.php
@@ -319,7 +319,7 @@ public function testChangeColumnWithForeignKey()
         $refTable = new \Phinx\Db\Table('ref_table', array(), $this->adapter);
         $refTable->addColumn('field1', 'string')->save();
 
-        $table = new \Phinx\Db\Table('table', array(), $this->adapter);
+        $table = new \Phinx\Db\Table('another_table', array(), $this->adapter);
         $table->addColumn('ref_table_id', 'integer')->save();
 
         $fk = new \Phinx\Db\Table\ForeignKey();
@@ -376,14 +376,28 @@ public function testChangeColumnWithCommasInCommentsOrDefaultValue()
         $this->assertEquals("'another default'", $rows[1]['dflt_value']);
     }
 
-    public function testDropColumn()
+    /**
+     * @dataProvider columnCreationArgumentProvider
+     */
+    public function testDropColumn($columnCreationArgs)
     {
         $table = new \Phinx\Db\Table('t', array(), $this->adapter);
-        $table->addColumn('column1', 'string')
-              ->save();
-        $this->assertTrue($this->adapter->hasColumn('t', 'column1'));
-        $this->adapter->dropColumn('t', 'column1');
-        $this->assertFalse($this->adapter->hasColumn('t', 'column1'));
+        $columnName = $columnCreationArgs[0];
+        call_user_func_array([$table, 'addColumn'], $columnCreationArgs);
+        $table->save();
+        $this->assertTrue($this->adapter->hasColumn('t', $columnName));
+
+        $this->adapter->dropColumn('t', $columnName);
+
+        $this->assertFalse($this->adapter->hasColumn('t', $columnName));
+    }
+
+    public function columnCreationArgumentProvider()
+    {
+        return [
+            [ ['column1', 'string'] ],
+            [ ['profile_colour', 'enum', ['values' => ['blue', 'red', 'white']]] ]
+        ];
     }
 
     public function testGetColumns()
@@ -528,9 +542,11 @@ public function testAddForeignKeyWithPdoExceptionErrorMode()
     public function testDropForeignKey()
     {
         $refTable = new \Phinx\Db\Table('ref_table', array(), $this->adapter);
-        $refTable->addColumn('field1', 'string')->save();
+        $refTable->addColumn('field1', 'string')
+                 ->addIndex(array('field1'), array('unique' => true))
+                 ->save();
 
-        $table = new \Phinx\Db\Table('table', array(), $this->adapter);
+        $table = new \Phinx\Db\Table('another_table', array(), $this->adapter);
         $table->addColumn('ref_table_id', 'integer')->addColumn('ref_table_field', 'string')->save();
 
         $fk = new \Phinx\Db\Table\ForeignKey();

diff --git a/tests/Phinx/Migration/Manager/EnvironmentTest.php b/tests/Phinx/Migration/Manager/EnvironmentTest.php
@@ -13,6 +13,7 @@ class PDOMock extends \PDO
 
     public function __construct()
     {
+
     }
 
     public function getAttribute($attribute)
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,6 +13,7 @@ class PDOMock extends \PDO @@
         public function __construct()
         {
         }
         public function getAttribute($attribute)
@@ Expand Down @@