Skip to content

Commit

Permalink
Merge pull request doctrine#11045 from greg0ire/dynamic-recommendation
Browse files Browse the repository at this point in the history
Dynamically resolve AUTO to SEQUENCE or IDENTITY
  • Loading branch information
greg0ire authored Nov 8, 2023
2 parents 73288bc + ffbe567 commit 1f62233
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 14 deletions.
10 changes: 5 additions & 5 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ Make sure to use the former when writing a type declaration or an `instanceof` c
To keep PHP mapping attributes consistent, order of arguments passed to above attributes has been changed
so `$targetEntity` is a first argument now. This change affects only non-named arguments usage.

## BC BREAK: AUTO keyword for identity generation defaults to IDENTITY for PostgreSQL now
## BC BREAK: AUTO keyword for identity generation defaults to IDENTITY for PostgreSQL when using `doctrine/dbal` 4

When using the AUTO strategy to let Doctrine determine the identity generation mecehanism for
an entity, PostgreSQL now uses IDENTITY instead of SEQUENCE. When upgrading from ORM 2.x
and preference is on keeping the SEQUENCE based identity generation, then configure the ORM
this way:
When using the `AUTO` strategy to let Doctrine determine the identity generation mechanism for
an entity, and when using `doctrine/dbal` 4, PostgreSQL now uses `IDENTITY`
instead of `SEQUENCE`. When upgrading from ORM 2.x and preference is on keeping
the `SEQUENCE` based identity generation, then configure the ORM this way:

```php
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
Expand Down
6 changes: 3 additions & 3 deletions docs/en/reference/basic-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@ 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.
If you are using `doctrine/dbal` 4, 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
Expand Down
10 changes: 9 additions & 1 deletion lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use function in_array;
use function is_a;
use function is_subclass_of;
use function method_exists;
use function str_contains;
use function strlen;
use function strtolower;
Expand Down Expand Up @@ -616,7 +617,14 @@ private function determineIdGeneratorStrategy(AbstractPlatform $platform): int
}
}

foreach (self::NON_IDENTITY_DEFAULT_STRATEGY as $platformFamily => $strategy) {
$nonIdentityDefaultStrategy = self::NON_IDENTITY_DEFAULT_STRATEGY;

// DBAL 3
if (method_exists($platform, 'getIdentitySequenceName')) {
$nonIdentityDefaultStrategy[Platforms\PostgreSQLPlatform::class] = ClassMetadata::GENERATOR_TYPE_SEQUENCE;
}

foreach ($nonIdentityDefaultStrategy as $platformFamily => $strategy) {
if (is_a($platform, $platformFamily)) {
return $strategy;
}
Expand Down
9 changes: 9 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC832Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand All @@ -17,6 +18,8 @@
use Doctrine\Tests\OrmFunctionalTestCase;
use PHPUnit\Framework\Attributes\Group;

use function method_exists;

class DDC832Test extends OrmFunctionalTestCase
{
protected function setUp(): void
Expand All @@ -42,6 +45,12 @@ public function tearDown(): void
$sm->dropTable($platform->quoteIdentifier('TREE_INDEX'));
$sm->dropTable($platform->quoteIdentifier('INDEX'));
$sm->dropTable($platform->quoteIdentifier('LIKE'));

// DBAL 3
if ($platform instanceof PostgreSQLPlatform && method_exists($platform, 'getIdentitySequenceName')) {
$sm->dropSequence($platform->quoteIdentifier('INDEX_id_seq'));
$sm->dropSequence($platform->quoteIdentifier('LIKE_id_seq'));
}
}

#[Group('DDC-832')]
Expand Down
31 changes: 26 additions & 5 deletions tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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;
Expand Down Expand Up @@ -128,15 +128,36 @@ private function setUpCmfForPlatform(AbstractPlatform $platform, array $preferen
return $cmf;
}

public function testRelyingOnLegacyIdGenerationDefaultsIsOKIfItResultsInTheCurrentlyRecommendedStrategyBeingUsed(): void
public function testPostgresSticksWithSequencesWhenDbal3IsUsed(): void
{
if (! method_exists(AbstractPlatform::class, 'getIdentitySequenceName')) {
self::markTestSkipped('This test requires DBAL 3');
}

$cm = $this->createValidClassMetadata();
$cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
$cmf = $this->setUpCmfForPlatform(new OraclePlatform());
$cmf = $this->setUpCmfForPlatform(new PostgreSQLPlatform());
$cmf->setMetadataForClass($cm->name, $cm);

$this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/orm/issues/8893');
$cmf->getMetadataFor($cm->name);
$metadata = $cmf->getMetadataFor($cm->name);

self::assertSame(ClassMetadata::GENERATOR_TYPE_SEQUENCE, $metadata->generatorType);
}

public function testPostgresSwitchesToIdentityColumnsWhenDbal4IsUsed(): void
{
if (method_exists(AbstractPlatform::class, 'getIdentitySequenceName')) {
self::markTestSkipped('This test requires DBAL 4');
}

$cm = $this->createValidClassMetadata();
$cm->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
$cmf = $this->setUpCmfForPlatform(new PostgreSQLPlatform());
$cmf->setMetadataForClass($cm->name, $cm);

$metadata = $cmf->getMetadataFor($cm->name);

self::assertSame(ClassMetadata::GENERATOR_TYPE_IDENTITY, $metadata->generatorType);
}

public function testGetMetadataForThrowsExceptionOnUnknownCustomGeneratorClass(): void
Expand Down

0 comments on commit 1f62233

Please sign in to comment.