From f83121fa971abe481a5f3129821a28232f6bacd9 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 6 Sep 2021 14:39:42 -0700 Subject: [PATCH] Represent table column names as list --- UPGRADE.md | 5 +++ src/Platforms/SqlitePlatform.php | 9 +++--- src/Schema/Comparator.php | 8 +++-- src/Schema/Table.php | 32 +++---------------- tests/Functional/Platform/AlterColumnTest.php | 10 +++--- .../Functional/Platform/RenameColumnTest.php | 10 +++--- .../Schema/MySQLSchemaManagerTest.php | 21 ++++++------ tests/Platforms/SqlitePlatformTest.php | 8 ++--- 8 files changed, 46 insertions(+), 57 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 82570ccf69f..cc1ccdbba62 100644 --- a/UPGRADE.md +++ b/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. diff --git a/src/Platforms/SqlitePlatform.php b/src/Platforms/SqlitePlatform.php index b4bf8f8aeba..ae28b5284ff 100644 --- a/src/Platforms/SqlitePlatform.php +++ b/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) { diff --git a/src/Schema/Comparator.php b/src/Schema/Comparator.php index 447d07130ee..b815befaae2 100644 --- a/src/Schema/Comparator.php +++ b/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; diff --git a/src/Schema/Table.php b/src/Schema/Table.php index fe160404662..c2d212ea0a6 100644 --- a/src/Schema/Table.php +++ b/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 + * @return list */ 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); } /** diff --git a/tests/Functional/Platform/AlterColumnTest.php b/tests/Functional/Platform/AlterColumnTest.php index d506a0df4de..7712448960d 100644 --- a/tests/Functional/Platform/AlterColumnTest.php +++ b/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()); } } diff --git a/tests/Functional/Platform/RenameColumnTest.php b/tests/Functional/Platform/RenameColumnTest.php index edb49d5b2da..f5c0fda0be5 100644 --- a/tests/Functional/Platform/RenameColumnTest.php +++ b/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()); } } diff --git a/tests/Functional/Schema/MySQLSchemaManagerTest.php b/tests/Functional/Schema/MySQLSchemaManagerTest.php index 44475264368..1129e84b02f 100644 --- a/tests/Functional/Schema/MySQLSchemaManagerTest.php +++ b/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()) ); } diff --git a/tests/Platforms/SqlitePlatformTest.php b/tests/Platforms/SqlitePlatformTest.php index e902b751e33..2defb06998c 100644 --- a/tests/Platforms/SqlitePlatformTest.php +++ b/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)',