doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.91k stars 2.5k forks source link

Alternative DiscriminatorMap declaration #9664

Open michnovka opened 2 years ago

michnovka commented 2 years ago

Feature Request

I have a custom DBAL type and I wish to use it as a discriminator column for JOINED inheritance type

Q A
New Feature yes
RFC no
BC Break no

Summary

In my case I use custom type that works with enums. But any custom type using any non-scalar PHP value would cause the same issues:

#[ORM\InheritanceType("JOINED")]
#[ORM\DiscriminatorColumn(name:"type", type: PaymentGatewayType::class)]
#[ORM\DiscriminatorMap([
    PaymentGatewayType::PAYSSION => Payssion::class,
    PaymentGatewayType::PAYMENTASIA => PaymentAsia::class
])]
abstract class PaymentGateway
{

The issue is that while any type can be used for DiscriminatorColumn, the DiscriminatorMap definition accepts only format like

#[ORM\DiscriminatorMap([
    VALUE_OF_COLUMN => SUB_CLASS,
])]

The VALUE_OF_COLUMN is the actual PHP value used by the Type thats converted to DB value with Type::convertToDatabaseValue().

And here lies the problem

in PHP as of 8.1 we cannot use objects as array keys. Therefore no objects / enums can be used inside DiscriminatorMap solely for this limitation. The JoinedSubclassPersister handles the conversion through DBAL Type correctly, but the PHP limitation makes it impossible to use any non-scalar keys.

Proposed solution

I propose an alternative declaration of DiscriminatorMap, but keep the old one for simplicity and BC.

#[ORM\DiscriminatorMapItem(VALUE_OF_COLUMN, SUB_CLASS)]

So my code would look:

#[ORM\InheritanceType("JOINED")]
#[ORM\DiscriminatorColumn(name:"type", type: PaymentGatewayType::class)]
#[ORM\DiscriminatorMapItem(PaymentGatewayType::PAYSSION, Payssion::class)]
#[ORM\DiscriminatorMapItem(PaymentGatewayType::PAYMENTASIA, PaymentAsia::class])]
abstract class PaymentGateway
{

This would allow any custom type to be used as discriminator

derrabus commented 2 years ago

Sounds reasonable, especially if we want to support backed enumerations as discriminators.

michnovka commented 2 years ago

This will require redesign of internal ClassMetadataInfo::$discriminatorMap as right now it also uses same structure with array key being the actual discriminator. Thats a public property, so there is a BC break. Otherwise wed have to copy lot of code to handle this special case, to keep BC.

michnovka commented 2 years ago

@beberlei would like your input on this too, please.

michnovka commented 2 years ago

How about we call the getDatabaseValue when constructing the discriminator map? And then when it is inserted into SQL, we no longer need to do that. This would allow us to keep the same ClassMetadataInfo::$discriminatorMap . I dont think BC would be broken, as currently it cannot work for any non-scalar types, and those have the same PHP value as DB value

Gabb1995 commented 1 year ago

is it at all possible to have php 8.1 enums as discriminator maps? I tried using the ->value method but it gives Constant expiression contains invalid operations.

michnovka commented 1 year ago

@Gabb1995 this question does not belong here, but since you already asked, the answer is no. Using Enum::enumCase->value inside attributes is a PHP 8.2 feature.