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
[11.x] Enhance database migrations #51373
[11.x] Enhance database migrations #51373
Conversation
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
Is there a reason, you mixed the SQLite and migration execution order changes into one PR instead of two separate ones? |
I'm eager to see this merged! |
@shaedrich we can't add SQLite features without fixing the migration commands' orders. |
@hafezdivandari You could have created the SQLite PR as draft, added a comment concerning the PR merging order and have backmerged the |
@shaedrich It's fine ;) |
To be honest this just is too much for us to take on reviewing and maintaining at the moment and I'm not sure the return on investment is there for end users. Feels like something maybe better reserved for later this year or early next year for Laravel 12. |
@taylorotwell How are you guys dropping columns with foreign key in SQLite currently? I do not understand how this is not a major issue for a lot a Laravel applications. |
This PR does a lot of improvements and unlocks many possibilities on database migrations, also makes it easier to maintain!
New Features / Enhancements π
Extend SQLite support to 3.26+
Laravel currently requires SQLite 3.35+, this PR extends SQLite support to 3.26+
Add and Drop Foreign Keys on SQLite
SQLite doesn't support adding / dropping foreign keys with
alter table
command, but as suggested on the docs, you may recreate the table with arbitrary changes, that's what this PR do to implement these commands.This PR adds support for blueprint's
$table->foreign()
,$table->dropForeign()
methods and->constrained()
modifier on SQLite:Fixes #51318, SQLite doesn't allow dropping a column if it is used in a foreign key constraint, this PR makes it possible:
Add and Drop the primary key on SQLite
This PR adds support for blueprint's
$table->primary()
,$table->dropPrimary()
methods,->primary()
, and->primary(false)->change()
modifiers when modifying table on SQLite:Preserve the order of commands
This is the most important change on this PR; When updating table and compiling blueprint commands to SQL, the order of commands wasn't respected:
As discussed on #50925, this will causes issue, unexpected results and leads to error most of the time when updating table. For example, the following code raises
The 'username' column already exists!
SQL error on all databases, becauseadd column
command was compiled beforerename column
:This PR fixes this by preserving the order of commands as declared on the blueprint. You don't have to call each command in a separate
Schema::table
anymore that breaks migration transaction!Blueprint State for SQLite
As explained before, when updating table on SQLite, we have to recreate the table to do arbitrary changes. To do so we need to know the current state of the table on each command. This PR adds
BlueprintState
class to keep track of changes on the table. After this PR we are able to mix any combination of blueprint commands in a singleSchema::table
(as demonstrated on the added integration tests) and it works as expected in the same order!Notes
Unlike other database drivers, foreign keys don't have name on SQLite, so obviously you can't drop a foreign key by its name on SQLite:
We probably need to remove
withSqlite
trait onorchestral/testbench
package, it seems we don't need that anymore if it's merged?