Deprecate values that are not shaped like a GUID in GuidType - #7504
Open
GromNaN wants to merge 1 commit into
Open
Deprecate values that are not shaped like a GUID in GuidType#7504GromNaN wants to merge 1 commit into
GromNaN wants to merge 1 commit into
Conversation
GuidType extends StringType and converts nothing, so any value goes through. On a platform with a native GUID type, the driver rejects a malformed value, for instance PostgreSQL reporting an invalid input syntax for uuid. On a platform without one, the column is a plain CHAR(36) and holds anything. Deprecate handling a value that is not shaped like a GUID, in both conversion directions, so the type can reject it in 5.0 the way the uuid and ulid types of the Symfony Doctrine bridge already do.
Member
|
Personally, I don't think that this is the right change:
The current behavior where the target database rejects the write seems right to me, because the UUID format and constraints are database-specific. Uniform behavior across databases should be achieved by representing UUIDs as value objects and formatting them at binding time (similar to the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix symfony/symfony#17488
GuidTypeextendsStringTypeand converts nothing, so any value reaches the database as is.On a platform with a native GUID type, the driver is the one that rejects a malformed value. PostgreSQL answers
SQLSTATE[22P02]: Invalid text representation: invalid input syntax for uuid: "asdf", which surfaces as a driver exception rather than as a conversion exception. On a platform without a native GUID type, the column is a plainCHAR(36)and holds anything, so the same value is silently stored and read back.That inconsistency leaks into the ecosystem. In the Symfony Doctrine bridge, the
uuidandulidtypes refuse what they cannot convert (AbstractUidType), so a consumer can catch aConversionExceptionand report an invalid value. Theguidtype gives no such guarantee, which is why symfony/symfony#65497 proposed to re-implement a GUID shape check inORMQueryBuilderLoader.This patch deprecates handling a value that is not shaped like a GUID, in both conversion directions. The value is still returned unchanged in 4.5. The exception is announced for 5.0, in line with how making a type stricter was handled before (
BC BREAK: Stricter DateTime types,BC BREAK: BIGINT values are cast to int if possible).The accepted shape is 32 hexadecimal digits, with optional surrounding braces and an optional hyphen after any group of four digits. That is everything PostgreSQL accepts for its native
uuidtype, so no value the database could have stored is reported as malformed. Braces must be balanced.The case to watch when 5.0 lands: an application storing something other than a GUID in a
guidcolumn on a platform without a native type. Such a column should use thestringtype instead.