Merge pull request #666 from deeky666/DBAL-924

[DBAL-923] [DBAL-924] Fix SQLite integer type primary autoincrement columns

lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

@@ -20,6 +20,7 @@
 namespace Doctrine\DBAL\Platforms;
 
 use Doctrine\DBAL\DBALException;
+use Doctrine\DBAL\Schema\Column;
 use Doctrine\DBAL\Schema\TableDiff;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Schema\ForeignKeyConstraint;
@@ -213,6 +214,11 @@ class SqlitePlatform extends AbstractPlatform
      */
     public function getBigIntTypeDeclarationSQL(array $field)
     {
+        //  SQLite autoincrement is implicit for INTEGER PKs, but not for BIGINT fields.
+        if ( ! empty($field['autoincrement'])) {
+            return $this->getIntegerTypeDeclarationSQL($field);
+        }
+
         return 'BIGINT' . $this->_getCommonIntegerTypeDeclarationSQL($field);
     }
 
@@ -221,6 +227,11 @@ class SqlitePlatform extends AbstractPlatform
      */
     public function getTinyIntTypeDeclarationSql(array $field)
     {
+        //  SQLite autoincrement is implicit for INTEGER PKs, but not for TINYINT fields.
+        if ( ! empty($field['autoincrement'])) {
+            return $this->getIntegerTypeDeclarationSQL($field);
+        }
+
         return 'TINYINT' . $this->_getCommonIntegerTypeDeclarationSQL($field);
     }
 
@@ -229,6 +240,11 @@ class SqlitePlatform extends AbstractPlatform
      */
     public function getSmallIntTypeDeclarationSQL(array $field)
     {
+        //  SQLite autoincrement is implicit for INTEGER PKs, but not for SMALLINT fields.
+        if ( ! empty($field['autoincrement'])) {
+            return $this->getIntegerTypeDeclarationSQL($field);
+        }
+
         return 'SMALLINT' . $this->_getCommonIntegerTypeDeclarationSQL($field);
     }
 
@@ -237,6 +253,11 @@ class SqlitePlatform extends AbstractPlatform
      */
     public function getMediumIntTypeDeclarationSql(array $field)
     {
+        //  SQLite autoincrement is implicit for INTEGER PKs, but not for MEDIUMINT fields.
+        if ( ! empty($field['autoincrement'])) {
+            return $this->getIntegerTypeDeclarationSQL($field);
+        }
+
         return 'MEDIUMINT' . $this->_getCommonIntegerTypeDeclarationSQL($field);
     }
 
@@ -845,6 +866,29 @@ class SqlitePlatform extends AbstractPlatform
      */
     private function getSimpleAlterTableSQL(TableDiff $diff)
     {
+        // Suppress changes on integer type autoincrement columns.
+        foreach ($diff->changedColumns as $oldColumnName => $columnDiff) {
+            if ( ! $columnDiff->fromColumn instanceof Column ||
+                ! $columnDiff->column instanceof Column ||
+                ! $columnDiff->column->getAutoincrement() ||
+                ! (string) $columnDiff->column->getType() === 'Integer'
+            ) {
+                continue;
+            }
+
+            if ( ! $columnDiff->hasChanged('type') && $columnDiff->hasChanged('unsigned')) {
+                unset($diff->changedColumns[$oldColumnName]);
+
+                continue;
+            }
+
+            $fromColumnType = (string) $columnDiff->fromColumn->getType();
+
+            if ($fromColumnType === 'SmallInt' || $fromColumnType === 'BigInt') {
+                unset($diff->changedColumns[$oldColumnName]);
+            }
+        }
+
         if ( ! empty($diff->renamedColumns) || ! empty($diff->addedForeignKeys) || ! empty($diff->addedIndexes)
                 || ! empty($diff->changedColumns) || ! empty($diff->changedForeignKeys) || ! empty($diff->changedIndexes)
                 || ! empty($diff->removedColumns) || ! empty($diff->removedForeignKeys) || ! empty($diff->removedIndexes)

tests/Doctrine/Tests/DBAL/Functional/Schema/SqliteSchemaManagerTest.php

@@ -131,4 +131,44 @@ EOS
         $this->assertArrayHasKey('primary', $tableIndexes, 'listTableIndexes() has to return a "primary" array key.');
         $this->assertEquals(array('other_id', 'id'), array_map('strtolower', $tableIndexes['primary']->getColumns()));
     }
+
+    /**
+     * @dataProvider getDiffListIntegerAutoincrementTableColumnsData
+     * @group DBAL-924
+     */
+    public function testDiffListIntegerAutoincrementTableColumns($integerType, $unsigned, $expectedComparatorDiff)
+    {
+        $tableName = 'test_int_autoincrement_table';
+
+        $offlineTable = new \Doctrine\DBAL\Schema\Table($tableName);
+        $offlineTable->addColumn('id', $integerType, array('autoincrement' => true, 'unsigned' => $unsigned));
+        $offlineTable->setPrimaryKey(array('id'));
+
+        $this->_sm->dropAndCreateTable($offlineTable);
+
+        $onlineTable = $this->_sm->listTableDetails($tableName);
+        $comparator = new Schema\Comparator();
+        $diff = $comparator->diffTable($offlineTable, $onlineTable);
+
+        if ($expectedComparatorDiff) {
+            $this->assertEmpty($this->_sm->getDatabasePlatform()->getAlterTableSQL($diff));
+        } else {
+            $this->assertFalse($diff);
+        }
+    }
+
+    /**
+     * @return array
+     */
+    public function getDiffListIntegerAutoincrementTableColumnsData()
+    {
+        return array(
+            array('smallint', false, true),
+            array('smallint', true, true),
+            array('integer', false, false),
+            array('integer', true, true),
+            array('bigint', false, true),
+            array('bigint', true, true),
+        );
+    }
 }

tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php

@@ -73,6 +73,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
 
     /**
      * @group DBAL-752
+     * @group DBAL-924
      */
     public function testGeneratesTypeDeclarationForTinyIntegers()
     {
@@ -81,11 +82,11 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
             $this->_platform->getTinyIntTypeDeclarationSQL(array())
         );
         $this->assertEquals(
-            'TINYINT',
+            'INTEGER',
             $this->_platform->getTinyIntTypeDeclarationSQL(array('autoincrement' => true))
         );
         $this->assertEquals(
-            'TINYINT',
+            'INTEGER',
             $this->_platform->getTinyIntTypeDeclarationSQL(
                 array('autoincrement' => true, 'primary' => true))
         );
@@ -101,6 +102,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
 
     /**
      * @group DBAL-752
+     * @group DBAL-924
      */
     public function testGeneratesTypeDeclarationForSmallIntegers()
     {
@@ -109,11 +111,15 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
             $this->_platform->getSmallIntTypeDeclarationSQL(array())
         );
         $this->assertEquals(
-            'SMALLINT',
+            'INTEGER',
             $this->_platform->getSmallIntTypeDeclarationSQL(array('autoincrement' => true))
         );
         $this->assertEquals(
-            'SMALLINT',
+            'INTEGER',
+            $this->_platform->getTinyIntTypeDeclarationSQL(array('autoincrement' => true, 'unsigned' => true))
+        );
+        $this->assertEquals(
+            'INTEGER',
             $this->_platform->getSmallIntTypeDeclarationSQL(
                 array('autoincrement' => true, 'primary' => true))
         );
@@ -129,6 +135,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
 
     /**
      * @group DBAL-752
+     * @group DBAL-924
      */
     public function testGeneratesTypeDeclarationForMediumIntegers()
     {
@@ -137,11 +144,15 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
             $this->_platform->getMediumIntTypeDeclarationSQL(array())
         );
         $this->assertEquals(
-            'MEDIUMINT',
+            'INTEGER',
             $this->_platform->getMediumIntTypeDeclarationSQL(array('autoincrement' => true))
         );
         $this->assertEquals(
-            'MEDIUMINT',
+            'INTEGER',
+            $this->_platform->getMediumIntTypeDeclarationSQL(array('autoincrement' => true, 'unsigned' => true))
+        );
+        $this->assertEquals(
+            'INTEGER',
             $this->_platform->getMediumIntTypeDeclarationSQL(
                 array('autoincrement' => true, 'primary' => true))
         );
@@ -165,6 +176,10 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
             'INTEGER',
             $this->_platform->getIntegerTypeDeclarationSQL(array('autoincrement' => true))
         );
+        $this->assertEquals(
+            'INTEGER',
+            $this->_platform->getIntegerTypeDeclarationSQL(array('autoincrement' => true, 'unsigned' => true))
+        );
         $this->assertEquals(
             'INTEGER',
             $this->_platform->getIntegerTypeDeclarationSQL(
@@ -182,6 +197,7 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
 
     /**
      * @group DBAL-752
+     * @group DBAL-924
      */
     public function testGeneratesTypeDeclarationForBigIntegers()
     {
@@ -190,11 +206,15 @@ class SqlitePlatformTest extends AbstractPlatformTestCase
             $this->_platform->getBigIntTypeDeclarationSQL(array())
         );
         $this->assertEquals(
-            'BIGINT',
+            'INTEGER',
             $this->_platform->getBigIntTypeDeclarationSQL(array('autoincrement' => true))
         );
         $this->assertEquals(
-            'BIGINT',
+            'INTEGER',
+            $this->_platform->getBigIntTypeDeclarationSQL(array('autoincrement' => true, 'unsigned' => true))
+        );
+        $this->assertEquals(
+            'INTEGER',
             $this->_platform->getBigIntTypeDeclarationSQL(
                 array('autoincrement' => true, 'primary' => true))
         );