From b2e042789588b0ffec31f68cffa647ba567cfabe Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 25 Jun 2021 09:40:53 -0700 Subject: [PATCH] [GH-4687] Remove support for Connection::lastInsertId($name) --- UPGRADE.md | 4 + src/Connection.php | 20 +---- src/Driver/Connection.php | 4 +- src/Driver/IBMDB2/Connection.php | 11 +-- src/Driver/Mysqli/Connection.php | 11 +-- src/Driver/OCI8/Connection.php | 22 +----- src/Driver/PDO/Connection.php | 15 +--- src/Driver/PDO/SQLSrv/Connection.php | 17 +---- src/Driver/SQLSrv/Connection.php | 18 +---- src/Portability/Connection.php | 13 +--- .../Functional/Driver/OCI8/ConnectionTest.php | 45 ----------- tests/Functional/Ticket/DBAL630Test.php | 14 ++-- tests/Functional/WriteTest.php | 75 +++---------------- 13 files changed, 38 insertions(+), 231 deletions(-) delete mode 100644 tests/Functional/Driver/OCI8/ConnectionTest.php diff --git a/UPGRADE.md b/UPGRADE.md index 397df3f1b3a..4b2ccf255e2 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,10 @@ awareness about deprecated code. # Upgrade to 4.0 +## Removed support for `Connection::lastInsertId($name)` + +The `Connection::lastInsertId()` method no longer accepts a sequence name. + ## Removed defaults for MySQL table charset, collation and engine The library no longer provides the default values for MySQL table charset, collation and engine. diff --git a/src/Connection.php b/src/Connection.php index b2b3d8d4d30..8435bcfc2fd 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -32,7 +32,6 @@ use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\DBAL\SQL\Parser; use Doctrine\DBAL\Types\Type; -use Doctrine\Deprecations\Deprecation; use Throwable; use Traversable; @@ -1104,31 +1103,20 @@ public function getTransactionNestingLevel(): int } /** - * Returns the ID of the last inserted row, or the last value from a sequence object, - * depending on the underlying driver. + * Returns the ID of the last inserted row. * * Note: This method may not return a meaningful or consistent result across different drivers, * because the underlying database may not even support the notion of AUTO_INCREMENT/IDENTITY - * columns or sequences. - * - * @param string|null $name Name of the sequence object from which the ID should be returned. + * columns. * * @return string A string representation of the last inserted ID. * * @throws Exception */ - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name !== null) { - Deprecation::trigger( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - } - try { - return $this->getWrappedConnection()->lastInsertId($name); + return $this->getWrappedConnection()->lastInsertId(); } catch (Driver\Exception $e) { throw $this->convertException($e); } diff --git a/src/Driver/Connection.php b/src/Driver/Connection.php index e094dadc3d1..73c46c1e8d4 100644 --- a/src/Driver/Connection.php +++ b/src/Driver/Connection.php @@ -37,11 +37,11 @@ public function quote(string $value): string; public function exec(string $sql): int; /** - * Returns the ID of the last inserted row or sequence value. + * Returns the ID of the last inserted row. * * @throws Exception */ - public function lastInsertId(?string $name = null): string; + public function lastInsertId(): string; /** * Initiates a transaction. diff --git a/src/Driver/IBMDB2/Connection.php b/src/Driver/IBMDB2/Connection.php index c711c0b38d7..5eb350cfc19 100644 --- a/src/Driver/IBMDB2/Connection.php +++ b/src/Driver/IBMDB2/Connection.php @@ -11,7 +11,6 @@ use Doctrine\DBAL\Driver\Result as ResultInterface; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\Statement as DriverStatement; -use Doctrine\Deprecations\Deprecation; use stdClass; use function assert; @@ -103,16 +102,8 @@ public function exec(string $sql): int return db2_num_rows($stmt); } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name !== null) { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - } - return db2_last_insert_id($this->conn); } diff --git a/src/Driver/Mysqli/Connection.php b/src/Driver/Mysqli/Connection.php index cd7ec5c6c02..4a491e15bf8 100644 --- a/src/Driver/Mysqli/Connection.php +++ b/src/Driver/Mysqli/Connection.php @@ -10,7 +10,6 @@ use Doctrine\DBAL\Driver\Result as ResultInterface; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\Statement as DriverStatement; -use Doctrine\Deprecations\Deprecation; use mysqli; use function assert; @@ -126,16 +125,8 @@ public function exec(string $sql): int return $this->conn->affected_rows; } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name !== null) { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - } - return (string) $this->conn->insert_id; } diff --git a/src/Driver/OCI8/Connection.php b/src/Driver/OCI8/Connection.php index 153fe549c01..87d4dc7c4eb 100644 --- a/src/Driver/OCI8/Connection.php +++ b/src/Driver/OCI8/Connection.php @@ -8,11 +8,9 @@ use Doctrine\DBAL\Driver\Exception\IdentityColumnsNotSupported; use Doctrine\DBAL\Driver\OCI8\Exception\ConnectionFailed; use Doctrine\DBAL\Driver\OCI8\Exception\Error; -use Doctrine\DBAL\Driver\OCI8\Exception\SequenceDoesNotExist; use Doctrine\DBAL\Driver\Result as ResultInterface; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\Statement as DriverStatement; -use Doctrine\Deprecations\Deprecation; use function addcslashes; use function assert; @@ -97,25 +95,9 @@ public function exec(string $sql): int return $this->prepare($sql)->execute()->rowCount(); } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name === null) { - throw IdentityColumnsNotSupported::new(); - } - - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - - $result = $this->query('SELECT ' . $name . '.CURRVAL FROM DUAL')->fetchOne(); - - if ($result === false) { - throw SequenceDoesNotExist::new(); - } - - return $result; + throw IdentityColumnsNotSupported::new(); } public function beginTransaction(): void diff --git a/src/Driver/PDO/Connection.php b/src/Driver/PDO/Connection.php index a96f47d2dd2..e6942c6386a 100644 --- a/src/Driver/PDO/Connection.php +++ b/src/Driver/PDO/Connection.php @@ -8,7 +8,6 @@ use Doctrine\DBAL\Driver\Result as ResultInterface; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\Statement as StatementInterface; -use Doctrine\Deprecations\Deprecation; use PDO; use PDOException; use PDOStatement; @@ -89,20 +88,10 @@ public function quote(string $value): string return $this->connection->quote($value); } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { try { - if ($name === null) { - return $this->connection->lastInsertId(); - } - - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - - return $this->connection->lastInsertId($name); + return $this->connection->lastInsertId(); } catch (PDOException $exception) { throw Exception::new($exception); } diff --git a/src/Driver/PDO/SQLSrv/Connection.php b/src/Driver/PDO/SQLSrv/Connection.php index 6c8732178b5..db18ca387be 100644 --- a/src/Driver/PDO/SQLSrv/Connection.php +++ b/src/Driver/PDO/SQLSrv/Connection.php @@ -8,7 +8,6 @@ use Doctrine\DBAL\Driver\Result; use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\Statement as StatementInterface; -use Doctrine\Deprecations\Deprecation; use PDO; final class Connection implements ServerInfoAwareConnection @@ -43,21 +42,9 @@ public function exec(string $sql): int return $this->connection->exec($sql); } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name === null) { - return $this->connection->lastInsertId($name); - } - - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - - return $this->prepare('SELECT CONVERT(VARCHAR(MAX), current_value) FROM sys.sequences WHERE name = ?') - ->execute([$name]) - ->fetchOne(); + return $this->connection->lastInsertId(); } public function beginTransaction(): void diff --git a/src/Driver/SQLSrv/Connection.php b/src/Driver/SQLSrv/Connection.php index 2cd1e37c6ae..a6be6eabb8a 100644 --- a/src/Driver/SQLSrv/Connection.php +++ b/src/Driver/SQLSrv/Connection.php @@ -9,7 +9,6 @@ use Doctrine\DBAL\Driver\ServerInfoAwareConnection; use Doctrine\DBAL\Driver\SQLSrv\Exception\Error; use Doctrine\DBAL\Driver\Statement as DriverStatement; -use Doctrine\Deprecations\Deprecation; use function sqlsrv_begin_transaction; use function sqlsrv_commit; @@ -87,22 +86,9 @@ public function exec(string $sql): int return $rowsAffected; } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name !== null) { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - - $result = $this->prepare('SELECT CONVERT(VARCHAR(MAX), current_value) FROM sys.sequences WHERE name = ?') - ->execute([$name]); - } else { - $result = $this->query('SELECT @@IDENTITY'); - } - - return $result->fetchOne(); + return $this->query('SELECT @@IDENTITY')->fetchOne(); } public function beginTransaction(): void diff --git a/src/Portability/Connection.php b/src/Portability/Connection.php index e9e908e6e13..3ec0b7ecf58 100644 --- a/src/Portability/Connection.php +++ b/src/Portability/Connection.php @@ -7,7 +7,6 @@ use Doctrine\DBAL\Driver\Connection as ConnectionInterface; use Doctrine\DBAL\Driver\Result as DriverResult; use Doctrine\DBAL\Driver\Statement as DriverStatement; -use Doctrine\Deprecations\Deprecation; /** * Portability wrapper for a Connection. @@ -58,17 +57,9 @@ public function exec(string $sql): int return $this->connection->exec($sql); } - public function lastInsertId(?string $name = null): string + public function lastInsertId(): string { - if ($name !== null) { - Deprecation::triggerIfCalledFromOutside( - 'doctrine/dbal', - 'https://github.com/doctrine/dbal/issues/4687', - 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' - ); - } - - return $this->connection->lastInsertId($name); + return $this->connection->lastInsertId(); } public function beginTransaction(): void diff --git a/tests/Functional/Driver/OCI8/ConnectionTest.php b/tests/Functional/Driver/OCI8/ConnectionTest.php deleted file mode 100644 index dbb6f0db493..00000000000 --- a/tests/Functional/Driver/OCI8/ConnectionTest.php +++ /dev/null @@ -1,45 +0,0 @@ -connection->getDriver() instanceof Driver) { - return; - } - - self::markTestSkipped('oci8 only test.'); - } - - public function testLastInsertIdAcceptsFqn(): void - { - $platform = $this->connection->getDatabasePlatform(); - $schemaManager = $this->connection->createSchemaManager(); - - $table = new Table('DBAL2595'); - $table->addColumn('id', 'integer', ['autoincrement' => true]); - $table->addColumn('foo', 'integer'); - - $schemaManager->dropAndCreateTable($table); - - $this->connection->executeStatement('INSERT INTO DBAL2595 (foo) VALUES (1)'); - - $schema = $this->connection->getDatabase(); - $sequence = $platform->getIdentitySequenceName($schema . '.DBAL2595', 'id'); - - self::assertEquals(1, $this->connection->lastInsertId($sequence)); - } -} diff --git a/tests/Functional/Ticket/DBAL630Test.php b/tests/Functional/Ticket/DBAL630Test.php index bdc3c4d6b6a..7a8456da0be 100644 --- a/tests/Functional/Ticket/DBAL630Test.php +++ b/tests/Functional/Ticket/DBAL630Test.php @@ -25,8 +25,8 @@ protected function setUp(): void } try { - $this->connection->executeStatement('CREATE TABLE dbal630 (id SERIAL, bool_col BOOLEAN NOT NULL);'); - $this->connection->executeStatement('CREATE TABLE dbal630_allow_nulls (id SERIAL, bool_col BOOLEAN);'); + $this->connection->executeStatement('CREATE TABLE dbal630 (id SERIAL, bool_col BOOLEAN NOT NULL)'); + $this->connection->executeStatement('CREATE TABLE dbal630_allow_nulls (id SERIAL, bool_col BOOLEAN)'); } catch (Exception $e) { } @@ -47,7 +47,7 @@ protected function tearDown(): void public function testBooleanConversionSqlLiteral(): void { $this->connection->executeStatement('INSERT INTO dbal630 (bool_col) VALUES(false)'); - $id = $this->connection->lastInsertId('dbal630_id_seq'); + $id = $this->connection->lastInsertId(); self::assertNotEmpty($id); $row = $this->connection->fetchAssociative('SELECT bool_col FROM dbal630 WHERE id = ?', [$id]); @@ -63,7 +63,7 @@ public function testBooleanConversionBoolParamRealPrepares(): void ['false'], [ParameterType::BOOLEAN] ); - $id = $this->connection->lastInsertId('dbal630_id_seq'); + $id = $this->connection->lastInsertId(); self::assertNotEmpty($id); $row = $this->connection->fetchAssociative('SELECT bool_col FROM dbal630 WHERE id = ?', [$id]); @@ -84,7 +84,7 @@ public function testBooleanConversionBoolParamEmulatedPrepares(): void $stmt->bindValue(1, $platform->convertBooleansToDatabaseValue('false'), ParameterType::BOOLEAN); $stmt->execute(); - $id = $this->connection->lastInsertId('dbal630_id_seq'); + $id = $this->connection->lastInsertId(); self::assertNotEmpty($id); @@ -111,7 +111,7 @@ public function testBooleanConversionNullParamEmulatedPrepares( $stmt->bindValue(1, $platform->convertBooleansToDatabaseValue($statementValue)); $stmt->execute(); - $id = $this->connection->lastInsertId('dbal630_allow_nulls_id_seq'); + $id = $this->connection->lastInsertId(); self::assertNotEmpty($id); @@ -142,7 +142,7 @@ public function testBooleanConversionNullParamEmulatedPreparesWithBooleanTypeInB ); $stmt->execute(); - $id = $this->connection->lastInsertId('dbal630_allow_nulls_id_seq'); + $id = $this->connection->lastInsertId(); self::assertNotEmpty($id); diff --git a/tests/Functional/WriteTest.php b/tests/Functional/WriteTest.php index fd1e4be4cf4..2f783f3fc7d 100644 --- a/tests/Functional/WriteTest.php +++ b/tests/Functional/WriteTest.php @@ -7,14 +7,10 @@ use DateTime; use Doctrine\DBAL\Exception\DriverException; use Doctrine\DBAL\ParameterType; -use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Tests\FunctionalTestCase; use Throwable; -use function array_filter; -use function strtolower; - class WriteTest extends FunctionalTestCase { protected function setUp(): void @@ -151,51 +147,21 @@ public function testLastInsertId(): void } self::assertEquals(1, $this->connection->insert('write_table', ['test_int' => 2, 'test_string' => 'bar'])); - $num = $this->lastInsertId(); + $num = $this->connection->lastInsertId(); self::assertGreaterThan(0, $num, 'LastInsertId() should be non-negative number.'); } - public function testLastInsertIdSequence(): void + public function testLastInsertIdNotSupported(): void { - if (! $this->connection->getDatabasePlatform()->supportsSequences()) { - self::markTestSkipped('Test only works on platforms with sequences.'); - } - - $sequence = new Sequence('write_table_id_seq'); - try { - $this->connection->createSchemaManager()->createSequence($sequence); - } catch (Throwable $e) { - } - - $sequences = $this->connection->createSchemaManager()->listSequences(); - self::assertCount(1, array_filter($sequences, static function ($sequence): bool { - return strtolower($sequence->getName()) === 'write_table_id_seq'; - })); - - $nextSequenceVal = $this->connection->fetchOne( - $this->connection->getDatabasePlatform()->getSequenceNextValSQL('write_table_id_seq') - ); - - $lastInsertId = $this->lastInsertId('write_table_id_seq'); - - self::assertGreaterThan(0, $lastInsertId); - self::assertEquals($nextSequenceVal, $lastInsertId); - } - - public function testLastInsertIdNoSequenceGiven(): void - { - if ( - ! $this->connection->getDatabasePlatform()->supportsSequences() - || $this->connection->getDatabasePlatform()->supportsIdentityColumns() - ) { + if ($this->connection->getDatabasePlatform()->supportsIdentityColumns()) { self::markTestSkipped( - "Test only works consistently on platforms that support sequences and don't support identity columns." + "Test only works consistently on platforms that don't support identity columns." ); } $this->expectException(DriverException::class); - $this->lastInsertId(); + $this->connection->lastInsertId(); } public function testInsertWithKeyValueTypes(): void @@ -269,9 +235,9 @@ public function testEmptyIdentityInsert(): void { $platform = $this->connection->getDatabasePlatform(); - if (! ($platform->supportsIdentityColumns() || $platform->usesSequenceEmulatedIdentityColumns())) { + if (! $platform->supportsIdentityColumns()) { self::markTestSkipped( - 'Test only works on platforms with identity columns or sequence emulated identity columns.' + 'Test only works on platforms with identity columns.' ); } @@ -288,19 +254,15 @@ public function testEmptyIdentityInsert(): void $this->connection->executeStatement($sql); } - $seqName = $platform->usesSequenceEmulatedIdentityColumns() - ? $platform->getIdentitySequenceName('test_empty_identity', 'id') - : null; - $sql = $platform->getEmptyIdentityInsertSQL('test_empty_identity', 'id'); $this->connection->executeStatement($sql); - $firstId = $this->lastInsertId($seqName); + $firstId = $this->connection->lastInsertId(); $this->connection->executeStatement($sql); - $secondId = $this->lastInsertId($seqName); + $secondId = $this->connection->lastInsertId(); self::assertGreaterThan($firstId, $secondId); } @@ -345,23 +307,4 @@ public function testDeleteWhereIsNull(): void self::assertCount(0, $data); } - - /** - * Returns the ID of the last inserted row or skips the test if the currently used driver - * doesn't support this feature - * - * @throws DriverException - */ - private function lastInsertId(?string $name = null): string - { - try { - return $this->connection->lastInsertId($name); - } catch (DriverException $e) { - if ($e->getSQLState() === 'IM001') { - self::markTestSkipped($e->getMessage()); - } - - throw $e; - } - } }