Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Squash migrations and migrate db to utf8mb4 #1804

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from
Open

Squash migrations and migrate db to utf8mb4 #1804

wants to merge 21 commits into from

Conversation

arthurpar06
Copy link
Contributor

@arthurpar06 arthurpar06 commented May 12, 2024 •
edited
Loading

This PR squashes all migrations into a single .sql file, enabling faster migration and a cleaner migrations folder. As we were performing some inserts into the migrations, I relocated them to seeds (base.yml).

This can be merged immediately, but it's advisable to ensure that we won't add new migrations until the release, as it would be preferable to have an empty migrations folder in the release.

Commands:

# For testing
php artisan migrate --database=testing # to have a sqlite db to dump
php artisan schema:dump --database=testing
# then rename testing-schema.sql to memory-schema.sql


# For mysql (use --prune to remove the migrations from the migrations folder)
php artisan schema:dump --prune

Closes #1731

@arthurpar06
Copy link
Contributor Author

Okay, what's great about the PR regarding internal fields is that it allowed me to test what happens when we re-dump the db after adding a migration. Verdict: it works perfectly; Laravel keeps the old dump and adds the migrations present in the migrations folder to create a single dump.

@arthurpar06 arthurpar06 changed the title Squash migrations Squash migrations and migrate db to utf8mb4 Jul 9, 2024
@arthurpar06
Copy link
Contributor Author

arthurpar06 commented Jul 9, 2024 •
edited
Loading

Important Notes Regarding This PR

Firstly, this PR squashes migrations. It is crucial to note that anyone who wants to upgrade to this version must first upgrade to the version immediately before this one and run all the migrations. This is because ALL migrations including those that were never run are now added to the dump, which is executed only when the database is empty. Therefore, it is VERY IMPORTANT that users upgrade to the previous version before upgrading to this one.

Additionally, this update changes the database charset to utf8mb4. As discussed with Nabeel, this will enable the addition of new features in the future, and it is better to make this change now so that all databases created from this release will not need to be converted later.
Before performing this update, it is VERY IMPORTANT to back up your database. Everything is automated and should go smoothly, but you can never be too careful.

I have tested with different databases containing varying amounts of data and did not encounter any issues with the migration. However, I would greatly appreciate feedback from @nabeelio, @FatihKoz and @ArthurHetem and anyone else who would like to test it. If you could test it locally with some of your databases and let me know if it works well, that would be great. Even though the change is fairly straightforward in theory.

Reference:

https://stackoverflow.com/questions/42704637/laravel-5-4-upgrade-converting-to-utf4mb-from-utf8

@FatihKoz
Copy link
Contributor

FatihKoz commented Jul 9, 2024 via email

@arthurpar06
Copy link
Contributor Author

By previous version, I mean having run all the latest migrations that were merged into the dev branch before this PR. I was thinking about the zip on Discord. What I mean is that you shouldn't catch up on 15 versions at once by applying this PR, and the thing is, I don't have much of an alternative for that...

Our last migration was just last week... So basically, if they are on the current dev and move to this dev, there won't be any problems. If people don't do this, they will end up missing migrations...

I'd suggest maybe posting a message on Discord, clearly explaining that they need to upgrade to the latest in two steps if people are catching up from a long time ago.

Regarding what happens if the table is already in utf8mb4, there is a check at the beginning of the migration that performs the update. It will not attempt the update if it's already utf8mb4 :)

@ArthurHetem
Copy link
Contributor

Everything worked fine here on my production data (in test env), there are no problems.

@FatihKoz
Copy link
Contributor

FatihKoz commented Jul 10, 2024 via email

@arthurpar06
Copy link
Contributor Author

So after discussing it with @nabeelio, the final solution we decided on is to keep the migrations in addition to the squash for now (which solves the issue of catching up many updates at once), and we will remove them at the beginning of version 7.1 (to allow compatibility with Laravel 11).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Trim migrations before release
4 participants
@arthurpar06 @FatihKoz @ArthurHetem @nabeelio