Deprecate reliance on non-optimal defaults

What was optimal 10 years ago no longer is, and things might change in
the future. Using AUTO is still the best solution in most cases, and it
should be easy to make it mean something else when it is not.

UPGRADE.md

@@ -1,5 +1,33 @@
 # 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.
@@ -14,7 +42,7 @@ Ensure `Doctrine\ORM\Configuration::setLazyGhostObjectEnabled(true)` is called t
 ## Deprecated accepting duplicate IDs in the identity map
 
 For any given entity class and ID value, there should be only one object instance
-representing the entity. 
+representing the entity.
 
 In https://github.com/doctrine/orm/pull/10785, a check was added that will guard this
 in the identity map. The most probable cause for violations of this rule are collisions
@@ -48,12 +76,12 @@ persister call `Doctrine\ORM\UnitOfWork::assignPostInsertId()` instead.
 In ORM 3.0, a change will be made regarding how the `AttributeDriver` reports field mappings.
 This change is necessary to be able to detect (and reject) some invalid mapping configurations.
 
-To avoid surprises during 2.x upgrades, the new mode is opt-in. It can be activated on the 
+To avoid surprises during 2.x upgrades, the new mode is opt-in. It can be activated on the
 `AttributeDriver` and `AnnotationDriver` by setting the `$reportFieldsWhereDeclared`
 constructor parameter to `true`. It will cause `MappingException`s to be thrown when invalid
 configurations are detected.
 
-Not enabling the new mode will cause a deprecation notice to be raised. In ORM 3.0, 
+Not enabling the new mode will cause a deprecation notice to be raised. In ORM 3.0,
 only the new mode will be available.
 
 # Upgrade to 2.15
@@ -73,7 +101,7 @@ and will be an error in 3.0.
 
 ## Deprecated undeclared entity inheritance
 
-As soon as an entity class inherits from another entity class, inheritance has to 
+As soon as an entity class inherits from another entity class, inheritance has to
 be declared by adding the appropriate configuration for the root entity.
 
 ## Deprecated stubs for "concrete table inheritance"

docs/en/reference/basic-mapping.rst

@@ -422,9 +422,25 @@ the field that serves as the identifier with the ``#[Id]`` attribute.
             # fields here
 
 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
+
+    <?php
+    use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+    use Doctrine\ORM\Configuration;
+
+    $config = new Configuration();
+    $config->setIdentityGenerationPreferences([
+        PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+    ]);
 
 .. _identifier-generation-strategies:
 
@@ -441,17 +457,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.
 -  ``UUID`` (deprecated): Tells Doctrine to use the built-in Universally
    Unique Identifier generator. This strategy provides full portability.
 -  ``NONE``: Tells Doctrine that the identifiers are assigned (and

lib/Doctrine/ORM/Configuration.php

@@ -13,6 +13,7 @@
 use Doctrine\Common\Cache\Psr6\CacheAdapter;
 use Doctrine\Common\Cache\Psr6\DoctrineProvider;
 use Doctrine\Common\Persistence\PersistentObject;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Cache\CacheConfiguration;
 use Doctrine\ORM\Cache\Exception\CacheException;
@@ -27,6 +28,7 @@
 use Doctrine\ORM\Exception\ProxyClassesAlwaysRegenerating;
 use Doctrine\ORM\Exception\UnknownEntityNamespace;
 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;
@@ -68,6 +70,21 @@ class Configuration extends \Doctrine\DBAL\Configuration
     /** @var mixed[] */
     protected $_attributes = [];
 
+    /** @psalm-var array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> */
+    private $identityGenerationPreferences = [];
+
+    /** @psalm-param array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> $value */
+    public function setIdentityGenerationPreferences(array $value): void
+    {
+        $this->identityGenerationPreferences = $value;
+    }
+
+    /** @psalm-return array<class-string<AbstractPlatform>, ClassMetadata::GENERATOR_TYPE_*> $value */
+    public function getIdentityGenerationPreferences(): array
+    {
+        return $this->identityGenerationPreferences;
+    }
+
     /**
      * Sets the directory where Doctrine generates any necessary proxy class files.
      *

lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

@@ -33,11 +33,13 @@
 
 use function assert;
 use function class_exists;
+use function constant;
 use function count;
 use function end;
 use function explode;
 use function get_class;
 use function in_array;
+use function is_a;
 use function is_subclass_of;
 use function str_contains;
 use function strlen;
@@ -71,6 +73,28 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory
     /** @var mixed[] */
     private $embeddablesActiveNesting = [];
 
+    private const LEGACY_DEFAULTS_FOR_ID_GENERATION = [
+        'Doctrine\DBAL\Platforms\MySqlPlatform' => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+        'Doctrine\DBAL\Platforms\PostgreSqlPlatform' => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
+        Platforms\DB2Platform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+        Platforms\MySQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+        Platforms\OraclePlatform::class => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
+        Platforms\PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_SEQUENCE,
+        Platforms\SQLServerPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+        Platforms\SqlitePlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+    ];
+
+    private const RECOMMENDED_STRATEGY = [
+        'Doctrine\DBAL\Platforms\MySqlPlatform' => 'IDENTITY',
+        'Doctrine\DBAL\Platforms\PostgreSqlPlatform' => 'IDENTITY',
+        Platforms\DB2Platform::class => 'IDENTITY',
+        Platforms\MySQLPlatform::class => 'IDENTITY',
+        Platforms\OraclePlatform::class => 'SEQUENCE',
+        Platforms\PostgreSQLPlatform::class => 'IDENTITY',
+        Platforms\SQLServerPlatform::class => 'IDENTITY',
+        Platforms\SqlitePlatform::class => 'IDENTITY',
+    ];
+
     /** @return void */
     public function setEntityManager(EntityManagerInterface $em)
     {
@@ -725,14 +749,42 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $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;
+        assert($this->em !== null);
+        foreach ($this->em->getConfiguration()->getIdentityGenerationPreferences() as $platformFamily => $strategy) {
+            if (is_a($platform, $platformFamily)) {
+                return $strategy;
+            }
+        }
+
+        foreach (self::LEGACY_DEFAULTS_FOR_ID_GENERATION as $platformFamily => $strategy) {
+            if (is_a($platform, $platformFamily)) {
+                $recommendedStrategyName = self::RECOMMENDED_STRATEGY[$platformFamily];
+                if ($strategy !== constant('Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_' . $recommendedStrategyName)) {
+                    Deprecation::trigger(
+                        'doctrine/orm',
+                        'https://github.com/doctrine/orm/issues/8893',
+                        <<<'DEPRECATION'
+Relying on non-optimal defaults for ID generation is deprecated.
+Instead, configure identifier generation strategies explicitly through
+configuration.
+We currently recommend "%s" for "%s", so you should use
+$configuration->setIdentityGenerationPreferences([
+    "%s" => ClassMetadata::GENERATOR_TYPE_%s,
+]);
+DEPRECATION
+                        ,
+                        $recommendedStrategyName,
+                        $platformFamily,
+                        $platformFamily,
+                        $recommendedStrategyName
+                    );
+                }
+
+                return $strategy;
+            }
         }
 
         if ($platform->supportsIdentityColumns()) {

phpstan-dbal2.neon

@@ -54,6 +54,11 @@ parameters:
             count: 1
             path: lib/Doctrine/ORM/Query/AST/Functions/SubstringFunction.php
 
+        -
+            message: '#^Class Doctrine\\DBAL\\Platforms\\MySQLPlatform not found\.$#'
+            count: 2
+            path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
+
         # Symfony cache supports passing a key prefix to the clear method.
         - '/^Method Psr\\Cache\\CacheItemPoolInterface\:\:clear\(\) invoked with 1 parameter, 0 required\.$/'
 

psalm-baseline.xml

@@ -537,6 +537,7 @@
       <code>$class</code>
       <code>$class</code>
       <code><![CDATA[new $definition['class']()]]></code>
+      <code>$platformFamily</code>
     </ArgumentTypeCoercion>
     <DeprecatedClass>
       <code>new UuidGenerator()</code>

tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php

@@ -9,6 +9,8 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\OraclePlatform;
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use Doctrine\ORM\Configuration;
 use Doctrine\ORM\EntityManagerInterface;
@@ -151,6 +153,66 @@ public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void
         self::assertInstanceOf(CustomIdGenerator::class, $actual->idGenerator);
     }
 
+    public function testRelyingOnLegacyIdGenerationDefaultsIsDeprecatedIfItResultsInASuboptimalDefault(): void
+    {
+        $cm1 = $this->createValidClassMetadata();
+        $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
+        $cmf    = new ClassMetadataFactoryTestSubject();
+        $driver = $this->createMock(Driver::class);
+        $driver->method('getDatabasePlatform')
+            ->willReturn(new PostgreSQLPlatform());
+        $entityManager = $this->createEntityManager(
+            new MetadataDriverMock(),
+            new Connection([], $driver)
+        );
+        $cmf->setEntityManager($entityManager);
+        $cmf->setMetadataForClass($cm1->name, $cm1);
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
+        $cmf->getMetadataFor($cm1->name);
+    }
+
+    public function testSpecifyingIdGenerationStrategyThroughConfigurationFixesTheDeprecation(): void
+    {
+        $cm1 = $this->createValidClassMetadata();
+        $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
+        $cmf    = new ClassMetadataFactoryTestSubject();
+        $driver = $this->createMock(Driver::class);
+        $driver->method('getDatabasePlatform')
+            ->willReturn(new PostgreSQLPlatform());
+        $entityManager = $this->createEntityManager(
+            new MetadataDriverMock(),
+            new Connection([], $driver)
+        );
+        $cmf->setEntityManager($entityManager);
+        $cmf->setMetadataForClass($cm1->name, $cm1);
+
+        $entityManager->getConfiguration()->setIdentityGenerationPreferences([
+            PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+        ]);
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
+        $cmf->getMetadataFor($cm1->name);
+    }
+
+    public function testRelyingOnLegacyIdGenerationDefaultsIsOKIfItResultsInTheCurrentlyRecommendedStrategyBeingUsed(): void
+    {
+        $cm1 = $this->createValidClassMetadata();
+        $cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
+        $cmf    = new ClassMetadataFactoryTestSubject();
+        $driver = $this->createMock(Driver::class);
+        $driver->method('getDatabasePlatform')
+            ->willReturn(new OraclePlatform());
+        $entityManager = $this->createEntityManager(
+            new MetadataDriverMock(),
+            new Connection([], $driver)
+        );
+        $cmf->setEntityManager($entityManager);
+        $cmf->setMetadataForClass($cm1->name, $cm1);
+
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
+        $cmf->getMetadataFor($cm1->name);
+    }
+
     public function testGetMetadataForThrowsExceptionOnUnknownCustomGeneratorClass(): void
     {
         $cm1 = $this->createValidClassMetadata();