Support for 3rd party enum types broken #7072

Open
opened 2026-01-22 15:44:08 +01:00 by admin · 1 comment
Owner

Originally created by @ovrflo on GitHub (Nov 24, 2022).

BC Break Report

Q A
BC Break yes
Version 2.13.4

Summary

I'm using a 3rd party implementation for PHP 8.1 enums (technically, a fork of https://github.com/bpolaszek/doctrine-native-enums ) and the upgrade to v2.13.4 suddenly broke changeset computation for my enums.

Looks like the 3 lines added in UnitOfWork in #10074 indiscriminately convert any original data backed enum into its value. This happens to work because in ClassMetadata, if enumType is used, the ReflectionProperty turns into ReflectionEnumProperty which extracts the value of the enum in question, not the enum instance, so things tend to work. My implementation, however, doesn't get to benefit from ReflectionEnumProperty.

I'm having some trouble following the code, but this is what I found.
I first encountered the error when trying to insert 2 entities (both using those Enums). The error was Invalid parameter number: number of bound variables does not match number of tokens. It would generate proper INSERTs but the changeset would be wrong (missing most fields).

It generates an initial changeset which is fine. It compares original data (NULL, since it's a new entity, persisted just now) with actual data (string value of my Enum), but somewhere along the way another changeset computation gets triggered and it somehow reaches the conclusion that only 2 out of 6 fields have changed and proceeds to using that changeset as insert data.

I know this is not the path provided by the ORM, but I still think that the ORM should support (as in "tolerate", try not to break) 3rd party implementations for Enum support. Personally, I like my current implementation since it allows me to properly define columns as #[Column(type: MyEnum::class) (not the hack-ish type: 'string', enumType: MyEnum::class) and it also allows me to use MySQL ENUM in the database (instead of string/char).

I downgraded to v2.13.3 and it works as it always did. I also tested v2.13.4 with the ORM's enumType and it also works as expected. It just doesn't work with v2.13.4 and custom enums.

I could try to fix this on my end. One way would be to just manually add the enumType metadata to any enum field (this is not a great option DX-wise). Another thing I could do would be to add a listener for loadClassMetadata that would do that (though, I'm not sure the event gets triggered at the right time, before wakeUpReflection). This is also not great performance-wise -- on each entity manager boot I would check all entities and patch the enums on-the-fly. I could also start using the ORM-supported enum implementation, but that has the downside of poorer DX and lacks support for MySQL ENUM.

Previous behavior

It would keep BackedEnums as they are and would always compare those when computing changesets.

Current behavior

When comparing changesets it converts BackedEnums to their value and it seems it compares a BackedEnum with a value of that BackedEnum since custom enum implementations don't benefit from ReflectionEnumProperty.

How to reproduce

Before attempting a reproducer I would like to hear some thoughts from you guys. I'd rather not waste time on a reproducer if this is deemed intended behavior.

Originally created by @ovrflo on GitHub (Nov 24, 2022). ### BC Break Report | Q | A |------------ | ------ | BC Break | yes | Version | 2.13.4 #### Summary I'm using a 3rd party implementation for PHP 8.1 enums (technically, a fork of https://github.com/bpolaszek/doctrine-native-enums ) and the upgrade to v2.13.4 suddenly broke changeset computation for my enums. Looks like the 3 lines added in UnitOfWork in #10074 indiscriminately convert any original data backed enum into its value. This happens to work because in `ClassMetadata`, if `enumType` is used, the `ReflectionProperty` turns into `ReflectionEnumProperty` which extracts the value of the enum in question, not the enum instance, so things tend to work. My implementation, however, doesn't get to benefit from `ReflectionEnumProperty`. I'm having some trouble following the code, but this is what I found. I first encountered the error when trying to insert 2 entities (both using those Enums). The error was `Invalid parameter number: number of bound variables does not match number of tokens`. It would generate proper `INSERTs` but the changeset would be wrong (missing most fields). It generates an initial changeset which is fine. It compares original data (`NULL`, since it's a new entity, `persisted` just now) with actual data (`string` value of my Enum), but somewhere along the way another changeset computation gets triggered and it somehow reaches the conclusion that only 2 out of 6 fields have changed and proceeds to using that changeset as insert data. I know this is not the path provided by the ORM, but I still think that the ORM should support (as in "tolerate", try not to break) 3rd party implementations for Enum support. Personally, I like my current implementation since it allows me to properly define columns as `#[Column(type: MyEnum::class)` (not the hack-ish `type: 'string', enumType: MyEnum::class`) and it also allows me to use MySQL `ENUM` in the database (instead of string/char). I downgraded to v2.13.3 and it works as it always did. I also tested v2.13.4 with the ORM's `enumType` and it also works as expected. It just doesn't work with v2.13.4 and custom enums. I could try to fix this on my end. One way would be to just manually add the `enumType` metadata to any enum field (this is not a great option DX-wise). Another thing I could do would be to add a listener for `loadClassMetadata` that would do that (though, I'm not sure the event gets triggered at the right time, before `wakeUpReflection`). This is also not great performance-wise -- on each entity manager boot I would check all entities and patch the enums on-the-fly. I could also start using the ORM-supported enum implementation, but that has the downside of poorer DX and lacks support for MySQL ENUM. #### Previous behavior It would keep `BackedEnums` as they are and would always compare those when computing changesets. #### Current behavior When comparing changesets it converts `BackedEnums` to their value and it seems it compares a `BackedEnum` with a value of that BackedEnum since custom enum implementations don't benefit from `ReflectionEnumProperty`. #### How to reproduce Before attempting a reproducer I would like to hear some thoughts from you guys. I'd rather not waste time on a reproducer if this is deemed intended behavior.
Author
Owner

@michnovka commented on GitHub (Dec 7, 2022):

@ovrflo thanks for trying to describe the issue, but I dont see how that PR could make these problems. If you can please make a repo with symfony project using your package and showing the error, on a minimal example, I will look into it and try to find a fix for you.

@michnovka commented on GitHub (Dec 7, 2022): @ovrflo thanks for trying to describe the issue, but I dont see how that PR could make these problems. If you can please make a repo with symfony project using your package and showing the error, on a minimal example, I will look into it and try to find a fix for you.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7072