-
Notifications
You must be signed in to change notification settings - Fork 11k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[10.x] Replace Deprecated DBAL Comparator creation with schema aware Comparator #46517
[10.x] Replace Deprecated DBAL Comparator creation with schema aware Comparator #46517
Conversation
I'll get those failing integration tests fixed this evening, just need to pop out for a few hours. |
@liamh101 What about framework/src/Illuminate/Database/Schema/Grammars/Grammar.php Lines 324 to 331 in 7d15f7e
|
@hafezdivandari Just need to do the |
@liamh101 isn't it better to change this function and use it on |
… constructor parameter
These are no longer included as provider in now schema aware when generating the SQL
@hafezdivandari can I do some further cleaning as a follow-up PR? I don't want to change too much and introduce additional bugs. My feeling is that this bug can go under the radar on many projects and have massive consequences, partially multi-tenant projects where fresh database migrations can happen regularly. |
@liamh101 sure, it was just a suggestion. But it seems that |
Renaming a column will work. However, changing a text type using the default Comparator won't work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging more into this @liamh101. I feel like we should give this a shot.
Can someone summarize all behavior changes / features / breaking changes / things we are not confident about for this PR in a single bullet point list? Please mark as ready for review when that is done. |
Sure!
No breaking changes were introduced in this PR, just a bug fix. Once this has been reviewed and merged, I will investigate other places Laravel might use deprecated DBAL functionality and replace them. This PR is to fix the migration issues introduced when upgrading DBAL from 2.x to 3.x. |
Thanks |
Reference to change in DBAL and why this needs to be done: doctrine/dbal#4746
This is currently causing this bug in Laravel where certain migration changes are ignored: #46492
Not sure if this is affecting other database types.
An example project of the existing bug can be found here: https://github.com/liamh101/laravel-column-bug