Merge pull request #4777 from morozov/issues/3589-column-names-as-list

Represent table columns as list in the order of declaration

UPGRADE.md

@@ -8,6 +8,11 @@ awareness about deprecated code.
 
 # Upgrade to 4.0
 
+## BC BREAK: Changes in the return value of `Table::getColumns()`
+
+1. The columns are returned as a list, not as an associative array.
+2. The columns are no longer sorted based on whether they belong to the primary key or a foreign key.
+
 ## BC BREAK: Removed schema comparison APIs that don't account for the current database connection and the database platform
 
 The `Schema::getMigrateFromSql()` and `::getMigrateToSql()` methods have been removed.

src/Platforms/SqlitePlatform.php

@@ -748,8 +748,8 @@ public function getAlterTableSQL(TableDiff $diff): array
         $newColumnNames = [];
         $columnSql      = [];
 
-        foreach ($table->getColumns() as $columnName => $column) {
-            $columnName                  = strtolower($columnName);
+        foreach ($table->getColumns() as $column) {
+            $columnName                  = strtolower($column->getName());
             $columns[$columnName]        = $column;
             $oldColumnNames[$columnName] = $newColumnNames[$columnName] = $column->getQuotedName($this);
         }
@@ -988,8 +988,9 @@ private function getColumnNamesInAlteredTable(TableDiff $diff, Table $fromTable)
     {
         $columns = [];
 
-        foreach ($fromTable->getColumns() as $columnName => $column) {
-            $columns[strtolower($columnName)] = $column->getName();
+        foreach ($fromTable->getColumns() as $column) {
+            $columnName                       = $column->getName();
+            $columns[strtolower($columnName)] = $columnName;
         }
 
         foreach ($diff->removedColumns as $columnName => $column) {

src/Schema/Comparator.php

@@ -194,7 +194,9 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff
         $toTableColumns   = $toTable->getColumns();
 
         /* See if all the columns in "from" table exist in "to" table */
-        foreach ($toTableColumns as $columnName => $column) {
+        foreach ($toTableColumns as $column) {
+            $columnName = strtolower($column->getName());
+
             if ($fromTable->hasColumn($columnName)) {
                 continue;
             }
@@ -204,7 +206,9 @@ public function diffTable(Table $fromTable, Table $toTable): ?TableDiff
         }
 
         /* See if there are any removed columns in "to" table */
-        foreach ($fromTableColumns as $columnName => $column) {
+        foreach ($fromTableColumns as $column) {
+            $columnName = strtolower($column->getName());
+
             // See if column is removed in "to" table.
             if (! $toTable->hasColumn($columnName)) {
                 $tableDifferences->removedColumns[$columnName] = $column;

src/Schema/Table.php

@@ -17,16 +17,13 @@
 use Doctrine\DBAL\Types\Type;
 
 use function array_filter;
-use function array_keys;
 use function array_merge;
-use function array_search;
-use function array_unique;
+use function array_values;
 use function in_array;
 use function is_string;
 use function preg_match;
 use function sprintf;
 use function strtolower;
-use function uksort;
 
 use const ARRAY_FILTER_USE_KEY;
 
@@ -474,34 +471,13 @@ public function removeUniqueConstraint(string $name): void
     }
 
     /**
-     * Returns ordered list of columns (primary keys are first, then foreign keys, then the rest)
+     * Returns the list of table columns.
      *
-     * @return array<string, Column>
+     * @return list<Column>
      */
     public function getColumns(): array
     {
-        $columns = $this->_columns;
-        $pkCols  = [];
-        $fkCols  = [];
-
-        $primaryKey = $this->getPrimaryKey();
-
-        if ($primaryKey !== null) {
-            $pkCols = $primaryKey->getColumns();
-        }
-
-        foreach ($this->getForeignKeys() as $fk) {
-            /** @var ForeignKeyConstraint $fk */
-            $fkCols = array_merge($fkCols, $fk->getColumns());
-        }
-
-        $colNames = array_unique(array_merge($pkCols, $fkCols, array_keys($columns)));
-
-        uksort($columns, static function (string $a, string $b) use ($colNames): int {
-            return array_search($a, $colNames, true) <=> array_search($b, $colNames, true);
-        });
-
-        return $columns;
+        return array_values($this->_columns);
     }
 
     /**

tests/Functional/Platform/AlterColumnTest.php

@@ -9,8 +9,6 @@
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 
-use function array_keys;
-
 class AlterColumnTest extends FunctionalTestCase
 {
     public function testColumnPositionRetainedAfterAltering(): void
@@ -32,7 +30,11 @@ public function testColumnPositionRetainedAfterAltering(): void
         self::assertNotNull($diff);
         $sm->alterTable($diff);
 
-        $table = $sm->listTableDetails('test_alter');
-        self::assertEquals(['c1', 'c2'], array_keys($table->getColumns()));
+        $table   = $sm->listTableDetails('test_alter');
+        $columns = $table->getColumns();
+
+        self::assertCount(2, $columns);
+        self::assertEqualsIgnoringCase('c1', $columns[0]->getName());
+        self::assertEqualsIgnoringCase('c2', $columns[1]->getName());
     }
 }

tests/Functional/Platform/RenameColumnTest.php

@@ -7,8 +7,6 @@
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Tests\FunctionalTestCase;
 
-use function array_keys;
-
 class RenameColumnTest extends FunctionalTestCase
 {
     public function testColumnPositionRetainedAfterRenaming(): void
@@ -29,7 +27,11 @@ public function testColumnPositionRetainedAfterRenaming(): void
         self::assertNotNull($diff);
         $sm->alterTable($diff);
 
-        $table = $sm->listTableDetails('test_rename');
-        self::assertSame(['c1_x', 'c2'], array_keys($table->getColumns()));
+        $table   = $sm->listTableDetails('test_rename');
+        $columns = $table->getColumns();
+
+        self::assertCount(2, $columns);
+        self::assertEqualsIgnoringCase('c1_x', $columns[0]->getName());
+        self::assertEqualsIgnoringCase('c2', $columns[1]->getName());
     }
 }

tests/Functional/Schema/MySQLSchemaManagerTest.php

@@ -306,41 +306,40 @@ public function testListLobTypeColumns(): void
 
         $this->schemaManager->dropAndCreateTable($table);
 
-        $platform       = $this->schemaManager->getDatabasePlatform();
-        $offlineColumns = $table->getColumns();
-        $onlineColumns  = $this->schemaManager->listTableColumns($tableName);
+        $platform      = $this->schemaManager->getDatabasePlatform();
+        $onlineColumns = $this->schemaManager->listTableColumns($tableName);
 
         self::assertSame(
-            $platform->getClobTypeDeclarationSQL($offlineColumns['col_tinytext']->toArray()),
+            $platform->getClobTypeDeclarationSQL($table->getColumn('col_tinytext')->toArray()),
             $platform->getClobTypeDeclarationSQL($onlineColumns['col_tinytext']->toArray())
         );
         self::assertSame(
-            $platform->getClobTypeDeclarationSQL($offlineColumns['col_text']->toArray()),
+            $platform->getClobTypeDeclarationSQL($table->getColumn('col_text')->toArray()),
             $platform->getClobTypeDeclarationSQL($onlineColumns['col_text']->toArray())
         );
         self::assertSame(
-            $platform->getClobTypeDeclarationSQL($offlineColumns['col_mediumtext']->toArray()),
+            $platform->getClobTypeDeclarationSQL($table->getColumn('col_mediumtext')->toArray()),
             $platform->getClobTypeDeclarationSQL($onlineColumns['col_mediumtext']->toArray())
         );
         self::assertSame(
-            $platform->getClobTypeDeclarationSQL($offlineColumns['col_longtext']->toArray()),
+            $platform->getClobTypeDeclarationSQL($table->getColumn('col_longtext')->toArray()),
             $platform->getClobTypeDeclarationSQL($onlineColumns['col_longtext']->toArray())
         );
 
         self::assertSame(
-            $platform->getBlobTypeDeclarationSQL($offlineColumns['col_tinyblob']->toArray()),
+            $platform->getBlobTypeDeclarationSQL($table->getColumn('col_tinyblob')->toArray()),
             $platform->getBlobTypeDeclarationSQL($onlineColumns['col_tinyblob']->toArray())
         );
         self::assertSame(
-            $platform->getBlobTypeDeclarationSQL($offlineColumns['col_blob']->toArray()),
+            $platform->getBlobTypeDeclarationSQL($table->getColumn('col_blob')->toArray()),
             $platform->getBlobTypeDeclarationSQL($onlineColumns['col_blob']->toArray())
         );
         self::assertSame(
-            $platform->getBlobTypeDeclarationSQL($offlineColumns['col_mediumblob']->toArray()),
+            $platform->getBlobTypeDeclarationSQL($table->getColumn('col_mediumblob')->toArray()),
             $platform->getBlobTypeDeclarationSQL($onlineColumns['col_mediumblob']->toArray())
         );
         self::assertSame(
-            $platform->getBlobTypeDeclarationSQL($offlineColumns['col_longblob']->toArray()),
+            $platform->getBlobTypeDeclarationSQL($table->getColumn('col_longblob')->toArray()),
             $platform->getBlobTypeDeclarationSQL($onlineColumns['col_longblob']->toArray())
         );
     }

tests/Platforms/SqlitePlatformTest.php

@@ -635,13 +635,13 @@ protected function getQuotesTableIdentifiersInAlterTableSQL(): array
         return [
             'DROP INDEX IDX_8C736521A81E660E',
             'DROP INDEX IDX_8C736521FDC58D6C',
-            'CREATE TEMPORARY TABLE __temp__foo AS SELECT fk, fk2, id, fk3, bar FROM "foo"',
+            'CREATE TEMPORARY TABLE __temp__foo AS SELECT id, fk, fk2, fk3, bar FROM "foo"',
             'DROP TABLE "foo"',
-            'CREATE TABLE "foo" (fk2 INTEGER NOT NULL, fk3 INTEGER NOT NULL, fk INTEGER NOT NULL, ' .
-            'war INTEGER NOT NULL, bar INTEGER DEFAULT NULL, bloo INTEGER NOT NULL, ' .
+            'CREATE TABLE "foo" (war INTEGER NOT NULL, fk INTEGER NOT NULL, fk2 INTEGER NOT NULL, ' .
+            'fk3 INTEGER NOT NULL, bar INTEGER DEFAULT NULL, bloo INTEGER NOT NULL, ' .
             'CONSTRAINT fk2 FOREIGN KEY (fk2) REFERENCES fk_table2 (id) NOT DEFERRABLE INITIALLY IMMEDIATE, ' .
             'CONSTRAINT fk_add FOREIGN KEY (fk3) REFERENCES fk_table (id) NOT DEFERRABLE INITIALLY IMMEDIATE)',
-            'INSERT INTO "foo" (fk, fk2, war, fk3, bar) SELECT fk, fk2, id, fk3, bar FROM __temp__foo',
+            'INSERT INTO "foo" (war, fk, fk2, fk3, bar) SELECT id, fk, fk2, fk3, bar FROM __temp__foo',
             'DROP TABLE __temp__foo',
             'ALTER TABLE "foo" RENAME TO "table"',
             'CREATE INDEX IDX_8C736521A81E660E ON "table" (fk)',