diff --git a/UPGRADE.md b/UPGRADE.md index ba124d6572..16a976f518 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -637,6 +637,34 @@ Use `toIterable()` instead. # Upgrade to 2.17 +## Deprecated: reliance on the non-optimal defaults that come with the `AUTO` identifier generation strategy + +When the `AUTO` identifier generation strategy was introduced, the best +strategy at the time was selected for each database platform. +A lot of time has passed since then, and support for better strategies has been +added. + +Because of that, it is now deprecated to rely on the historical defaults when +they differ from what we recommend now. + +Instead, you should pick a strategy for each database platform you use, and it +will be used when using `AUTO`. As of now, only PostgreSQL is affected by this. +It is recommended that PostgreSQL user configure their new applications to use +`IDENTITY`: + +```php +use Doctrine\DBAL\Platforms\PostgreSQLPlatform; +use Doctrine\ORM\Configuration; + +assert($configuration instanceof Configuration); +$configuration->setIdentityGenerationPreferences([ + PostgreSQLPlatform::CLASS => ClassMetadata::GENERATOR_TYPE_IDENTITY, +]); +``` + +If migrating an existing application is too costly, the deprecation can be +addressed by configuring `SEQUENCE` as the default strategy. + ## Deprecate `EntityManagerInterface::getPartialReference()` This method does not have a replacement and will be removed in 3.0. diff --git a/docs/en/reference/basic-mapping.rst b/docs/en/reference/basic-mapping.rst index 0e3f0210ee..818647786e 100644 --- a/docs/en/reference/basic-mapping.rst +++ b/docs/en/reference/basic-mapping.rst @@ -324,9 +324,25 @@ the field that serves as the identifier with the ``#[Id]`` attribute. In most cases using the automatic generator strategy (``#[GeneratedValue]``) is -what you want. It defaults to the identifier generation mechanism your current -database vendor prefers: AUTO_INCREMENT with MySQL, sequences with PostgreSQL -and Oracle and so on. +what you want, but for backwards-compatibility reasons it might not. It +defaults to the identifier generation mechanism your current database +vendor preferred at the time that strategy was introduced: +``AUTO_INCREMENT`` with MySQL, sequences with PostgreSQL and Oracle and +so on. +We now recommend using ``IDENTITY`` for PostgreSQL, and you can achieve +that while still using the ``AUTO`` strategy, by configuring what it +defaults to. + +.. code-block:: php + + setIdentityGenerationPreferences([ + PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY, + ]); .. _identifier-generation-strategies: @@ -343,17 +359,18 @@ Here is the list of possible generation strategies: - ``AUTO`` (default): Tells Doctrine to pick the strategy that is preferred by the used database platform. The preferred strategies - are IDENTITY for MySQL, SQLite, MsSQL and SQL Anywhere and SEQUENCE - for Oracle and PostgreSQL. This strategy provides full portability. -- ``SEQUENCE``: Tells Doctrine to use a database sequence for ID - generation. This strategy does currently not provide full - portability. Sequences are supported by Oracle, PostgreSql and - SQL Anywhere. + are ``IDENTITY`` for MySQL, SQLite, MsSQL and SQL Anywhere and, for + historical reasons, ``SEQUENCE`` for Oracle and PostgreSQL. This + strategy provides full portability. - ``IDENTITY``: Tells Doctrine to use special identity columns in the database that generate a value on insertion of a row. This strategy does currently not provide full portability and is supported by the following platforms: MySQL/SQLite/SQL Anywhere - (AUTO\_INCREMENT), MSSQL (IDENTITY) and PostgreSQL (SERIAL). + (``AUTO_INCREMENT``), MSSQL (``IDENTITY``) and PostgreSQL (``SERIAL``). +- ``SEQUENCE``: Tells Doctrine to use a database sequence for ID + generation. This strategy does currently not provide full + portability. Sequences are supported by Oracle, PostgreSql and + SQL Anywhere. - ``NONE``: Tells Doctrine that the identifiers are assigned (and thus generated) by your code. The assignment must take place before a new entity is passed to ``EntityManager#persist``. NONE is the diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 2bf45006d7..63c39d42de 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -4,9 +4,11 @@ namespace Doctrine\ORM; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\ORM\Cache\CacheConfiguration; use Doctrine\ORM\Exception\InvalidEntityRepository; use Doctrine\ORM\Internal\Hydration\AbstractHydrator; +use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataFactory; use Doctrine\ORM\Mapping\DefaultEntityListenerResolver; use Doctrine\ORM\Mapping\DefaultNamingStrategy; @@ -39,6 +41,21 @@ class Configuration extends \Doctrine\DBAL\Configuration /** @var mixed[] */ protected array $attributes = []; + /** @psalm-var array, ClassMetadata::GENERATOR_TYPE_*> */ + private $identityGenerationPreferences = []; + + /** @psalm-param array, ClassMetadata::GENERATOR_TYPE_*> $value */ + public function setIdentityGenerationPreferences(array $value): void + { + $this->identityGenerationPreferences = $value; + } + + /** @psalm-return array, ClassMetadata::GENERATOR_TYPE_*> $value */ + public function getIdentityGenerationPreferences(): array + { + return $this->identityGenerationPreferences; + } + /** * Sets the directory where Doctrine generates any necessary proxy class files. */ diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 60ec110644..ad782108a0 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -16,7 +16,6 @@ use Doctrine\ORM\Id\BigIntegerIdentityGenerator; use Doctrine\ORM\Id\IdentityGenerator; use Doctrine\ORM\Id\SequenceGenerator; -use Doctrine\ORM\Mapping\Exception\CannotGenerateIds; use Doctrine\ORM\Mapping\Exception\InvalidCustomGenerator; use Doctrine\ORM\Mapping\Exception\UnknownGeneratorType; use Doctrine\Persistence\Mapping\AbstractClassMetadataFactory; @@ -32,6 +31,7 @@ use function end; use function explode; use function in_array; +use function is_a; use function is_subclass_of; use function str_contains; use function strlen; @@ -55,6 +55,10 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory /** @var mixed[] */ private array $embeddablesActiveNesting = []; + private const NON_IDENTITY_DEFAULT_STRATEGY = [ + Platforms\OraclePlatform::class => ClassMetadata::GENERATOR_TYPE_SEQUENCE, + ]; + public function setEntityManager(EntityManagerInterface $em): void { $this->em = $em; @@ -599,25 +603,23 @@ private function completeIdGeneratorMapping(ClassMetadata $class): void } } - /** @psalm-return ClassMetadata::GENERATOR_TYPE_SEQUENCE|ClassMetadata::GENERATOR_TYPE_IDENTITY */ + /** @psalm-return ClassMetadata::GENERATOR_TYPE_* */ private function determineIdGeneratorStrategy(AbstractPlatform $platform): int { - if ( - $platform instanceof Platforms\OraclePlatform - || $platform instanceof Platforms\PostgreSQLPlatform - ) { - return ClassMetadata::GENERATOR_TYPE_SEQUENCE; - } - - if ($platform->supportsIdentityColumns()) { - return ClassMetadata::GENERATOR_TYPE_IDENTITY; + assert($this->em !== null); + foreach ($this->em->getConfiguration()->getIdentityGenerationPreferences() as $platformFamily => $strategy) { + if (is_a($platform, $platformFamily)) { + return $strategy; + } } - if ($platform->supportsSequences()) { - return ClassMetadata::GENERATOR_TYPE_SEQUENCE; + foreach (self::NON_IDENTITY_DEFAULT_STRATEGY as $platformFamily => $strategy) { + if (is_a($platform, $platformFamily)) { + return $strategy; + } } - throw CannotGenerateIds::withPlatform($platform); + return ClassMetadata::GENERATOR_TYPE_IDENTITY; } private function truncateSequenceName(string $schemaElementName): string diff --git a/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php b/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php deleted file mode 100644 index a9895ceb21..0000000000 --- a/lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php +++ /dev/null @@ -1,23 +0,0 @@ - + $platformFamily diff --git a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php index b13548560f..61aef5d404 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SchemaTool/PostgreSqlSchemaToolTest.php @@ -12,7 +12,6 @@ use Doctrine\ORM\Mapping\JoinColumn; use Doctrine\ORM\Mapping\ManyToOne; use Doctrine\ORM\Mapping\Table; -use Doctrine\Tests\Models\CMS\CmsAddress; use Doctrine\Tests\OrmFunctionalTestCase; use PHPUnit\Framework\Attributes\Group; @@ -32,13 +31,6 @@ protected function setUp(): void } } - public function testPostgresMetadataSequenceIncrementedBy10(): void - { - $address = $this->_em->getClassMetadata(CmsAddress::class); - - self::assertEquals(1, $address->sequenceGeneratorDefinition['allocationSize']); - } - #[Group('DDC-1657')] public function testUpdateSchemaWithPostgreSQLSchema(): void { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php index d3b908fd56..5cf9fb9d9c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php @@ -5,7 +5,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; use Doctrine\DBAL\Platforms\OraclePlatform; -use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Mapping\Column; use Doctrine\ORM\Mapping\DiscriminatorColumn; use Doctrine\ORM\Mapping\DiscriminatorMap; @@ -43,11 +42,6 @@ public function tearDown(): void $sm->dropTable($platform->quoteIdentifier('TREE_INDEX')); $sm->dropTable($platform->quoteIdentifier('INDEX')); $sm->dropTable($platform->quoteIdentifier('LIKE')); - - if ($platform instanceof PostgreSQLPlatform) { - $sm->dropSequence($platform->quoteIdentifier('INDEX_id_seq')); - $sm->dropSequence($platform->quoteIdentifier('LIKE_id_seq')); - } } #[Group('DDC-832')] diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index ec7dea7a83..752ad559b1 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManagerInterface; @@ -21,7 +22,6 @@ use Doctrine\ORM\Mapping\DiscriminatorColumn; use Doctrine\ORM\Mapping\DiscriminatorMap; use Doctrine\ORM\Mapping\Entity; -use Doctrine\ORM\Mapping\Exception\CannotGenerateIds; use Doctrine\ORM\Mapping\GeneratedValue; use Doctrine\ORM\Mapping\Id; use Doctrine\ORM\Mapping\InheritanceType; @@ -97,25 +97,6 @@ public function testGetMetadataForSingleClass(): void self::assertTrue($cmMap1->hasField('name')); } - public function testItThrowsWhenUsingAutoWithIncompatiblePlatform(): void - { - $cm1 = $this->createValidClassMetadata(); - $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO); - - $driver = $this->createMock(Driver::class); - $driver->method('getDatabasePlatform') - ->willReturn($this->createMock(AbstractPlatform::class)); - - $connection = new Connection([], $driver); - $entityManager = $this->createEntityManager(new MetadataDriverMock(), $connection); - $cmf = new ClassMetadataFactoryTestSubject(); - $cmf->setEntityManager($entityManager); - $cmf->setMetadataForClass($cm1->name, $cm1); - $this->expectException(CannotGenerateIds::class); - - $actual = $cmf->getMetadataFor($cm1->name); - } - public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void { $cm1 = $this->createValidClassMetadata(); @@ -130,6 +111,34 @@ public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void self::assertInstanceOf(CustomIdGenerator::class, $actual->idGenerator); } + /** @param array, ClassMetadata::GENERATOR_TYPE_*> $preferences */ + private function setUpCmfForPlatform(AbstractPlatform $platform, array $preferences = []): ClassMetadataFactoryTestSubject + { + $cmf = new ClassMetadataFactoryTestSubject(); + $driver = $this->createMock(Driver::class); + $driver->method('getDatabasePlatform') + ->willReturn($platform); + $entityManager = $this->createEntityManager( + new MetadataDriverMock(), + new Connection([], $driver), + ); + $cmf->setEntityManager($entityManager); + $entityManager->getConfiguration()->setIdentityGenerationPreferences($preferences); + + return $cmf; + } + + public function testRelyingOnLegacyIdGenerationDefaultsIsOKIfItResultsInTheCurrentlyRecommendedStrategyBeingUsed(): void + { + $cm = $this->createValidClassMetadata(); + $cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO); + $cmf = $this->setUpCmfForPlatform(new OraclePlatform()); + $cmf->setMetadataForClass($cm->name, $cm); + + $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893'); + $cmf->getMetadataFor($cm->name); + } + public function testGetMetadataForThrowsExceptionOnUnknownCustomGeneratorClass(): void { $cm1 = $this->createValidClassMetadata();