From 9ccd199a70504f9a89c1100bdd5eeac893e0b0a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Wed, 31 May 2023 23:08:56 +0200 Subject: [PATCH 1/6] Reference the right argument This deprecation references an argument that does not exist. --- UPGRADE.md | 6 +++--- src/Schema/TableDiff.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index d59c30bfcb8..44f14436dfc 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -260,9 +260,9 @@ if (! $diff->isEmpty()) { } ``` -## Deprecated not passing `$fromColumn` to the `TableDiff` constructor. +## Deprecated not passing `$fromTable` to the `TableDiff` constructor. -Not passing `$fromColumn` to the `TableDiff` constructor has been deprecated. +Not passing `$fromTable` to the `TableDiff` constructor has been deprecated. The `TableDiff::$name` property and the `TableDiff::getName()` method have been deprecated as well. In order to obtain the name of the table that the diff describes, use `TableDiff::getOldTable()`. @@ -1002,7 +1002,7 @@ deprecated in order to provide a more consistent API. ## Deprecated `Comparator::compare($fromSchema, $toSchema)` -The usage of `Comparator::compare($fromSchema, $toSchema)` is deprecated and +The usage of `Comparator::compare($fromSchema, $toSchema)` is deprecated and replaced by `Comparator::compareSchemas($fromSchema, $toSchema)` in order to clarify the purpose of the method. diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 58128d75b3e..4c6618c3c90 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -184,7 +184,7 @@ public function __construct( Deprecation::trigger( 'doctrine/dbal', 'https://github.com/doctrine/dbal/pull/5678', - 'Not passing the $fromColumn to %s is deprecated.', + 'Not passing the $fromTable to %s is deprecated.', __METHOD__, ); } From b94a87f5e06ee168db15f6749907dbfc41194d1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Wed, 31 May 2023 23:20:30 +0200 Subject: [PATCH 2/6] Invert condition It contradicts the message. --- src/Schema/TableDiff.php | 2 +- tests/Schema/TableDiffTest.php | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Schema/TableDiff.php b/src/Schema/TableDiff.php index 4c6618c3c90..9aaf9e7702f 100644 --- a/src/Schema/TableDiff.php +++ b/src/Schema/TableDiff.php @@ -180,7 +180,7 @@ public function __construct( $this->changedForeignKeys = $changedForeignKeys; $this->removedForeignKeys = $removedForeignKeys; - if ($fromTable !== null) { + if ($fromTable === null) { Deprecation::trigger( 'doctrine/dbal', 'https://github.com/doctrine/dbal/pull/5678', diff --git a/tests/Schema/TableDiffTest.php b/tests/Schema/TableDiffTest.php index 946f390b18b..b3349b293b1 100644 --- a/tests/Schema/TableDiffTest.php +++ b/tests/Schema/TableDiffTest.php @@ -6,11 +6,14 @@ use Doctrine\DBAL\Schema\Identifier; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class TableDiffTest extends TestCase { + use VerifyDeprecations; + /** @var AbstractPlatform&MockObject */ private AbstractPlatform $platform; @@ -53,4 +56,25 @@ public function testReturnsNewName(): void self::assertEquals(new Identifier('bar'), $tableDiff->getNewName()); } + + public function testOmittingFromTableInConstructorIsDeprecated(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/5678'); + $tableDiff = new TableDiff('foo'); + } + + public function testPassingFromTableToConstructorIsDeprecated(): void + { + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/dbal/pull/5678'); + $tableDiff = new TableDiff( + 'foo', + [], + [], + [], + [], + [], + [], + new Table('foo'), + ); + } } From aadddd535a17d9ba0cd5462567a3088a36dfcf74 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 5 Jun 2023 16:33:48 +0200 Subject: [PATCH 3/6] Fix deprecation layer in Table --- src/Schema/Table.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Schema/Table.php b/src/Schema/Table.php index 7fbd6091a46..ce4cc3260f1 100644 --- a/src/Schema/Table.php +++ b/src/Schema/Table.php @@ -724,7 +724,7 @@ public function getColumns() */ public function getForeignKeyColumns() { - Deprecation::trigger( + Deprecation::triggerIfCalledFromOutside( 'doctrine/dbal', 'https://github.com/doctrine/dbal/pull/5731', '%s is deprecated. Use getForeignKey() and ForeignKeyConstraint::getLocalColumns() instead.', @@ -813,7 +813,7 @@ public function getPrimaryKey() */ public function getPrimaryKeyColumns() { - Deprecation::trigger( + Deprecation::triggerIfCalledFromOutside( 'doctrine/dbal', 'https://github.com/doctrine/dbal/pull/5731', '%s is deprecated. Use getPrimaryKey() and Index::getColumns() instead.', From dbb880328f30b6774b02aa858b1f3c4d564d9700 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Tue, 13 Jun 2023 09:41:49 +0200 Subject: [PATCH 4/6] Run tests with PHP 8.3 (#6060) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PHP 8.3.0-alpha1 has been released. Let's see if our tests blow up. 💣 --- .github/workflows/continuous-integration.yml | 38 ++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index d1bab0a6233..5c9ae3a0554 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -41,6 +41,7 @@ jobs: - "8.0" - "8.1" - "8.2" + - "8.3" dependencies: - "highest" extension: @@ -106,8 +107,8 @@ jobs: matrix: php-version: - "7.4" - - "8.1" - "8.2" + - "8.3" oracle-version: - "21" include: @@ -164,8 +165,8 @@ jobs: matrix: php-version: - "7.4" - - "8.1" - "8.2" + - "8.3" oracle-version: - "21" include: @@ -229,16 +230,16 @@ jobs: - "pgsql" - "pdo_pgsql" include: - - php-version: "8.1" + - php-version: "8.2" postgres-version: "15" extension: "pgsql" - - php-version: "8.2" + - php-version: "8.3" postgres-version: "15" extension: "pgsql" - - php-version: "8.1" + - php-version: "8.2" postgres-version: "15" extension: "pdo_pgsql" - - php-version: "8.2" + - php-version: "8.3" postgres-version: "15" extension: "pdo_pgsql" @@ -302,22 +303,22 @@ jobs: - "mysqli" - "pdo_mysql" include: - - php-version: "8.1" + - php-version: "8.2" mariadb-version: "10.7" extension: "mysqli" - - php-version: "8.1" + - php-version: "8.2" mariadb-version: "10.7" extension: "pdo_mysql" - - php-version: "8.1" + - php-version: "8.2" mariadb-version: "10.11" extension: "mysqli" - - php-version: "8.1" + - php-version: "8.2" mariadb-version: "10.11" extension: "pdo_mysql" - - php-version: "8.2" + - php-version: "8.3" mariadb-version: "10.11" extension: "mysqli" - - php-version: "8.2" + - php-version: "8.3" mariadb-version: "10.11" extension: "pdo_mysql" @@ -371,7 +372,7 @@ jobs: matrix: php-version: - "7.4" - - "8.0" + - "8.1" mysql-version: - "5.7" - "8.0" @@ -390,20 +391,20 @@ jobs: php-version: "7.4" mysql-version: "8.0" extension: "mysqli" - - php-version: "8.1" + - php-version: "8.2" mysql-version: "8.0" extension: "mysqli" custom-entrypoint: >- --entrypoint sh mysql:8 -c "exec docker-entrypoint.sh mysqld --default-authentication-plugin=mysql_native_password" - - php-version: "8.1" + - php-version: "8.2" mysql-version: "8.0" extension: "pdo_mysql" custom-entrypoint: >- --entrypoint sh mysql:8 -c "exec docker-entrypoint.sh mysqld --default-authentication-plugin=mysql_native_password" - - php-version: "8.2" + - php-version: "8.3" mysql-version: "8.0" extension: "mysqli" - - php-version: "8.2" + - php-version: "8.3" mysql-version: "8.0" extension: "pdo_mysql" @@ -461,8 +462,8 @@ jobs: matrix: php-version: - "7.4" - - "8.1" - "8.2" + - "8.3" extension: - "sqlsrv" - "pdo_sqlsrv" @@ -529,6 +530,7 @@ jobs: php-version: - "7.4" - "8.2" + - "8.3" services: ibm_db2: From 4de9c19be88e7fe801618b6fe35a1708e8f43968 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 13 Jun 2023 10:51:04 +0200 Subject: [PATCH 5/6] trigger if called outside for AbstractPlatform::supportsCreateDropDatabase (#6064) | Q | A |------------- | ----------- | Type | improvement | Fixed issues | https://github.com/doctrine/dbal/issues/6063 #### Summary See https://github.com/doctrine/dbal/issues/6063. This makes sure we silence the deprecation if the method is called from within AbstractPlatform itself. --- src/Platforms/AbstractPlatform.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index 32e8b743c93..8e58a66b945 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -4155,7 +4155,7 @@ public function getDefaultSchemaName() */ public function supportsCreateDropDatabase() { - Deprecation::trigger( + Deprecation::triggerIfCalledFromOutside( 'doctrine/dbal', 'https://github.com/doctrine/dbal/pull/5513', '%s is deprecated.', From 19f0dec95edd6a3c3c5ff1d188ea94c6b7fc903f Mon Sep 17 00:00:00 2001 From: Franco Lombardo Date: Thu, 15 Jun 2023 09:40:12 +0200 Subject: [PATCH 6/6] Fix incorrect syntax for dropping primary indexes in PostgreSQL (#6025) | Q | A |------------- | ----------- | Type | bug/feature/improvement | Fixed issues | https://github.com/doctrine/dbal/issues/6024 #### Summary The correct way of dropping primary index in PostgreSQL is not dropping the index, but removing the primary key constraints --------- Co-authored-by: Alexander M. Turek --- src/Platforms/PostgreSQLPlatform.php | 31 ++++++++++++++++ tests/Functional/Driver/DBAL6024Test.php | 43 ++++++++++++++++++++++ tests/Platforms/PostgreSQLPlatformTest.php | 38 +++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 tests/Functional/Driver/DBAL6024Test.php diff --git a/src/Platforms/PostgreSQLPlatform.php b/src/Platforms/PostgreSQLPlatform.php index ba9427cc2b9..b01eb860a2f 100644 --- a/src/Platforms/PostgreSQLPlatform.php +++ b/src/Platforms/PostgreSQLPlatform.php @@ -10,6 +10,7 @@ use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\PostgreSQLSchemaManager; use Doctrine\DBAL\Schema\Sequence; +use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; use Doctrine\DBAL\Types\BinaryType; use Doctrine\DBAL\Types\BlobType; @@ -809,6 +810,36 @@ public function getDropForeignKeySQL($foreignKey, $table) return $this->getDropConstraintSQL($foreignKey, $table); } + /** + * {@inheritDoc} + */ + public function getDropIndexSQL($index, $table = null) + { + if ($index instanceof Index && $index->isPrimary() && $table !== null) { + $constraintName = $index->getName() === 'primary' ? $this->tableName($table) . '_pkey' : $index->getName(); + + return $this->getDropConstraintSQL($constraintName, $table); + } + + if ($index === '"primary"' && $table !== null) { + $constraintName = $this->tableName($table) . '_pkey'; + + return $this->getDropConstraintSQL($constraintName, $table); + } + + return parent::getDropIndexSQL($index, $table); + } + + /** + * @param Table|string|null $table + * + * @return string + */ + private function tableName($table) + { + return $table instanceof Table ? $table->getName() : (string) $table; + } + /** * {@inheritDoc} */ diff --git a/tests/Functional/Driver/DBAL6024Test.php b/tests/Functional/Driver/DBAL6024Test.php new file mode 100644 index 00000000000..4b6d3bf374f --- /dev/null +++ b/tests/Functional/Driver/DBAL6024Test.php @@ -0,0 +1,43 @@ +addColumn('id', 'integer'); + $table->setPrimaryKey(['id']); + $this->dropAndCreateTable($table); + + $newTable = clone $table; + $newTable->dropPrimaryKey(); + + $schemaManager = $this->connection->createSchemaManager(); + $diff = $schemaManager->createComparator()->compareTables($table, $newTable); + + $statements = $this->connection->getDatabasePlatform()->getAlterTableSQL($diff); + foreach ($statements as $statement) { + $this->connection->executeStatement($statement); + } + + $validationSchema = $schemaManager->introspectSchema(); + $validationTable = $validationSchema->getTable($table->getName()); + + $this->assertNull($validationTable->getPrimaryKey()); + } +} diff --git a/tests/Platforms/PostgreSQLPlatformTest.php b/tests/Platforms/PostgreSQLPlatformTest.php index b721a5c95fe..ac91e2c6687 100644 --- a/tests/Platforms/PostgreSQLPlatformTest.php +++ b/tests/Platforms/PostgreSQLPlatformTest.php @@ -581,6 +581,44 @@ public function testDroppingConstraintsBeforeColumns(): void self::assertEquals($expectedSql, $sql); } + public function testDroppingPrimaryKey(): void + { + $oldTable = new Table('mytable'); + $oldTable->addColumn('id', 'integer'); + $oldTable->setPrimaryKey(['id']); + + $newTable = clone $oldTable; + $newTable->dropPrimaryKey(); + + $diff = (new Comparator())->compareTables($oldTable, $newTable); + + $sql = $this->platform->getAlterTableSQL($diff); + + $expectedSql = ['ALTER TABLE mytable DROP CONSTRAINT mytable_pkey']; + + self::assertEquals($expectedSql, $sql); + } + + public function testDroppingPrimaryKeyWithUserDefinedName(): void + { + self::markTestSkipped('Edge case not covered yet'); + + $oldTable = new Table('mytable'); + $oldTable->addColumn('id', 'integer'); + $oldTable->setPrimaryKey(['id'], 'a_user_name'); + + $newTable = clone $oldTable; + $newTable->dropPrimaryKey(); + + $diff = (new Comparator())->compareTables($oldTable, $newTable); + + $sql = $this->platform->getAlterTableSQL($diff); + + $expectedSql = ['ALTER TABLE mytable DROP CONSTRAINT a_user_name']; + + self::assertEquals($expectedSql, $sql); + } + public function testUsesSequenceEmulatedIdentityColumns(): void { self::assertTrue($this->platform->usesSequenceEmulatedIdentityColumns());