IDENTITY identifier strategy for PostgreSQL breaks when inherited from MappedSuperclass

[DemoniacDeath]

BC Break Report

Q	A
BC Break	yes
Version	2.16.0

Summary

Persisting an @Entity which has it's @Id defined in it's parent (@MappedSuperclass) with IDENTITY generator strategy on PostgreSQL platform.
It seems that the breaking change occured in this PR #10455, specifically changes in ClassMetadataFactory that removed the condtion of $rootEntityFound for calling inheritIdGeneratorMapping()

Previous behavior

In version 2.15.5 the IdentityGenerator used to retrieve inserted id came from the Entity itself with $sequenceName matching the schema. So the value was retrieved from the correct sequence.

Current behavior

Now the IdentityGenerator used to retrieve inserted id comes from the parent @MappedSuperclass and $sequenceName is wrong (name of parent class + sequence suffix) and non-existent in schema, so a PostgreSQL error about relation not existing is thrown.

How to reproduce

E.g. (irrelevant code and domain-specific terminology is ommited):

/**
 * @ORM\MappedSuperclass
 */
abstract class AbstractTaskOperatorHistoryRecord
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    private ?int $id = null;
}

/**
 * @ORM\Entity()
 * @ORM\Table(name="foo_bar_task_operator_history")
 */
class FooBarTaskOperatorHistoryRecord extends AbstractTaskOperatorHistoryRecord
{
}

$this->_em->persist(new FooBarTaskOperatorHistoryRecord());
$this->_em->flush();

The PostgreSQL sequence used by id column in foo_bar_task_operator_history table is foo_bar_task_operator_history_id_seq.
The sequence used to retrieve inserted id was foo_bar_task_operator_history_id_seq before the breaking change. Now it is trying to get the value from abstracttaskoperatorhistoryrecord_id_seq which obviously does not and should not exist. The error thrown is

PDOException

SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "abstracttaskoperatorhistoryrecord_id_seq" does not exist

[pkly]

I've noticed a similar issue in one of our projects, running on MariaDB, similarly with a parent entity (DiscriminatorMap + Joined inheritance) but in our case I see:

An exception occurred while executing a query: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens

2.15.5 works as expected, 2.16.0 crashes on the same code.
We have a parent entity with a generated id value, the child entity also contains an automatically generated uuid value (via custom generator).

[julienbrunet21]

Hello I found a workaround which consists of changing the parent's id to protected instead of private

[DemoniacDeath]

Hello I found a workaround which consists of changing the parent's id to protected instead of private

changing id property from private to protected did not change the result of failing test case #10928 for me

[mpdude]

@DemoniacDeath did you opt into the new „report fields where declared“ setting?

[DemoniacDeath]

@mpdude

OrmFunctionalTestCase.php:803

        $config->setMetadataDriverImpl(
            $mappingDriver ?? ORMSetup::createDefaultAnnotationDriver([
                realpath(__DIR__ . '/Models/Cache'),
                realpath(__DIR__ . '/Models/GeoNames'),
            ], null, true)
        );

it is set to true in the failing test case (by default). changing it to false did not change the outcome.

AFAIUI the issue is specifically with the code in \Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata

[melroy89]

Mbin is also effected by this regression issue of Doctrine: MbinOrg/mbin#147

[mpdude]

I see two ways how this could be fixed.

The first one is in #11050. It reverts the way inheritance works for @SequenceGeneratorDefinition and mapped superclasses. But, with this change one cannot define @SequenceGeneratorDefinition on mapped superclasses and expect it to be inherited by child entity classes - at least not when using the new reportFieldsWhereDeclared mapping driver mode.

The second approach is in #11052. It will recognize when an explicit @SequenceGeneratorDefinition is used and pass that one on unchanged to child classes (= all use the same sequence name). Otherwise, the default one will be generated for every single class, so the sequence name is based on the table name.

[Chris53897]

@mpdude I have tested your solution. Thanks for your work. Looks like it works for me with 2.16.2 and report_fields_where_declared: true

@DemoniacDeath can you confirm this too?

[mpdude]

Please also test #11052.

[DemoniacDeath]

@Chris53897 @mpdude indeed, both solutions have resolved the issue

[Arkemlar]

Same problem here but in my case I use Embeddable that contains id field:

#[ORM\Embeddable]
class AutoincrementId extends BaseId implements Stringable
{
    #[ORM\Id, ORM\GeneratedValue(strategy: 'IDENTITY'), ORM\Column(type: 'bigint', nullable: false, options: ['unsigned' => true])]
    protected int $id;
}


#[ORM\Entity]
class ProductUnit
{
    #[ORM\Embedded(columnPrefix: false)]
    public readonly ProductUnitId $id;
}

It worked fine with strategy "AUTO", but with strategy "IDENTITY" it throws exception:
SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "product_unit_id_id_seq" does not exist CONTEXT: unnamed portal parameter $1 = '...'

Identity strategy so fucked, wasted lots of hours trying to make it work...

[greg0ire]

@Arkemlar please stay polite, no one wants to hear you complain like that, especially not people providing you with free software .

[Arkemlar]

@greg0ire I didn't mean anyone in person or group of people at all, only Identity strategy feature because it makes a lot of pain working with it.

[mpdude]

@Arkemlar do #11050 or #11052 fix your issue as well?

[Arkemlar]

@mpdude I tried both, but none fixed my issue :(
Doctrine generates incorrect sequence name product_unit_id_id_seq, while it must be product_unit_id_seq (doctrine:migration:diff generated name is product_unit_id_id_seq). I don't get how is it possible that migrations work properly while runtime not. I suppose it must use same mappings...

The error bumps up when doctrine's BigIntegerIdentityGenerator tries to get the id of freshly inserted row, see screenshot:

Debug panel screenshot

[mpdude]

@Arkemlar does your code work with, say, 2.14.x, but broke at or after 2.15.1?

If not (does not work before either) you're seeing an issue different from this one here.

[Arkemlar]

@mpdude nope, it is not working in any version unless I use custom id generator that removes sequenceName argument at all. But my problem is similar to topic starter's issue because it caused by invalid sequence name.

[mpdude]

Please create a different issue then so we can keep the discussion here focused.