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,32 @@
 # Upgrade to 2.12
 
+## 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 `Doctrine\ORM\Cache\MultiGetRegion`
 
 The interface will be merged with `Doctrine\ORM\Cache\Region` in 3.0.

docs/en/reference/basic-mapping.rst

@@ -358,9 +358,27 @@ annotation.
  # 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 Oracle, 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\OraclePlatform;
+ use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+ use Doctrine\ORM\Configuration;
+
+ $config = new Configuration();
+ $config->setIdentityGenerationPreferences([
+ OraclePlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+ PostgreSQLPlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+ ]);
 
 .. _identifier-generation-strategies:
 
@@ -377,17 +395,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\Proxy\AbstractProxyFactory;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Cache\CacheConfiguration;
 use Doctrine\ORM\Cache\Exception\CacheException;
@@ -26,6 +27,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;
@@ -58,6 +60,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

@@ -30,10 +30,12 @@
 
 use function assert;
 use function class_exists;
+use function constant;
 use function count;
 use function end;
 use function explode;
 use function in_array;
+use function is_a;
 use function is_subclass_of;
 use function strlen;
 use function strpos;
@@ -64,6 +66,32 @@ 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\DrizzlePlatform::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\SQLAnywherePlatform::class => ClassMetadata::GENERATOR_TYPE_IDENTITY,
+ 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\DrizzlePlatform::class => 'IDENTITY',
+ Platforms\MySQLPlatform::class => 'IDENTITY',
+ Platforms\OraclePlatform::class => 'SEQUENCE',
+ Platforms\PostgreSQLPlatform::class => 'IDENTITY',
+ Platforms\SQLAnywherePlatform::class => 'IDENTITY',
+ Platforms\SQLServerPlatform::class => 'IDENTITY',
+ Platforms\SqlitePlatform::class => 'IDENTITY',
+ ];
+
  /**
  * @return void
  */
@@ -643,22 +671,53 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $class): void
  }
 
  /**
- * @psalm-return ClassMetadata::GENERATOR_TYPE_SEQUENCE|ClassMetadata::GENERATOR_TYPE_IDENTITY
+ * @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()) {
  return ClassMetadata::GENERATOR_TYPE_IDENTITY;
  }
 
- if ($platform->supportsSequences()) {
+ if (
+ $platform instanceof Platforms\OraclePlatform
+ || $platform instanceof Platforms\PostgreSQLPlatform
+ ) {
  return ClassMetadata::GENERATOR_TYPE_SEQUENCE;
  }
 

phpstan-baseline.neon

@@ -235,6 +235,11 @@ parameters:
  count: 1
  path: lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php
 
+ -
+ message: "#^Class Doctrine\\\\DBAL\\\\Platforms\\\\DrizzlePlatform not found\\.$#"
+ count: 2
+ path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
+
  -
  message: "#^If condition is always true\\.$#"
  count: 2

phpstan-dbal2.neon

@@ -42,5 +42,10 @@ 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

@@ -653,10 +653,11 @@
  </PropertyNotSetInConstructor>
  </file>
  <file src="lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php">
- <ArgumentTypeCoercion occurrences="4">
+ <ArgumentTypeCoercion occurrences="5">
  <code>$class</code>
  <code>$class</code>
  <code>$nonSuperclassParents</code>
+ <code>$platformFamily</code>
  <code>new $definition['class']()</code>
  </ArgumentTypeCoercion>
  <DeprecatedClass occurrences="1">
@@ -704,6 +705,10 @@
  <PropertyTypeCoercion occurrences="1">
  <code>$subClass-&gt;table</code>
  </PropertyTypeCoercion>
+ <UndefinedClass occurrences="2">
+ <code>Platforms\DrizzlePlatform</code>
+ <code>Platforms\DrizzlePlatform</code>
+ </UndefinedClass>
  </file>
  <file src="lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php">
  <DeprecatedConstant occurrences="1">

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

@@ -8,6 +8,9 @@
 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;
 use Doctrine\ORM\Event\OnClassMetadataNotFoundEventArgs;
@@ -50,6 +53,8 @@
 
 class ClassMetadataFactoryTest extends OrmTestCase
 {
+ use VerifyDeprecations;
+
  public function testGetMetadataForSingleClass(): void
  {
  $platform = $this->createMock(AbstractPlatform::class);
@@ -123,6 +128,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();