Prevent column flagging as diff just because type differs (when one type is parent to other) #4016
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.
Summary
The ability to extend the dbal type system allows us to define our own Enum implementations. Often these extend from the StringType built-in type class. Everything works as it should except for one caveat - when generating migration diffs the Comparator::diffColumn() will detect that the two types are not identical and add the column to the generated migration diff every time. This leaves us with 2 main choices:
Although a perfectly reasonable solution, in some cases #2 is less than ideal (i.e., if we don't want to make modifications to the schema columns). For example, when porting a legacy app where the source of truth (and legacy migration system) is still the old app. In this case, we use the diff not to migrate the changes, but instead to make our schema continually match the legacy app while the migration work continues (i.e., the legacy app can and will still change).
To satisfy the above use case, I have presented this pull request. I have added a small modification to the Comparator::diffColumn() method which basically checks if the second check type is a subclass of the parent and that the parent is a basic string type. If this is the case, then the 'type' diff is not added; i.e., Only detected differences in options will then cause the schema diff to report change to the column
To summarise, I think implementing an enum as an extension of the string class is a very common case and I don't see a strong reason to enforce/require
requiresSQLCommentHint()
for the type in this case (Doctrine is perfectly able to detect and hydrate the type correctly at runtime)Kind regards,
Richard