Skip to content

[10.x] Let database handle default collation #6241

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
Merged
merged 1 commit into from
Sep 19, 2023
Merged

[10.x] Let database handle default collation #6241

merged 1 commit into from
Sep 19, 2023

Conversation

Jubeki
Copy link
Contributor

@Jubeki Jubeki commented Sep 18, 2023

I tested with Laravel Sail both MySQL and MariaDB both seem to accept the null value and then each use their own default collation.

Here what I did:

MySQL 

laravel new mysql-collation-test
cd mysql-collation-test

# Edit config/database.php and update collation in mysql to null

php artisan sail:insall
# Select only MySQL

sail up

# In another terminal
sail artisan migrate
sail artisan db

# SQL Command for showing collation defaults
SHOW VARIABLES LIKE 'collation%';

Result:

+----------------------+--------------------+
| Variable_name        | Value              |
+----------------------+--------------------+
| collation_connection | utf8mb4_0900_ai_ci |
| collation_database   | utf8mb4_0900_ai_ci |
| collation_server     | utf8mb4_0900_ai_ci |
+----------------------+--------------------+
3 rows in set (0.00 sec)

MariaDB 

laravel new mariadb-collation-test
cd mariadb-collation-test

# Edit config/database.php and update collation in mysql to null

php artisan sail:insall
# Select only MariaDB

sail up

# In another terminal
sail artisan migrate
sail artisan db

# SQL Command for showing collation defaults
SHOW VARIABLES LIKE 'collation%';

Result:

+----------------------+--------------------+
| Variable_name        | Value              |
+----------------------+--------------------+
| collation_connection | utf8mb4_general_ci |
| collation_database   | utf8mb4_general_ci |
| collation_server     | utf8mb4_general_ci |
+----------------------+--------------------+
3 rows in set (0.00 sec)

You can so configure your defaults outside of Laravel if wanted, while still having the option to explicitly set a default for your application.

Risks:

  • You have upgraded your Database and may have not updated your default collation. This could result in unexpected behaviour.
  • Different collations between development / production environments

Alternative: Duplicate the mysql entry to mariadb and then change the default collation.

  • Remaining TODOS: Update Docs and Sail installation for MariaDB
  • while this is not a breaking change for existing applications, some developers may be confused if mysql is suddenly changed to mariadb

@binaryfire does this solution also work in your case?

@Jubeki
Copy link
Contributor Author

Jubeki commented Sep 18, 2023 •
edited
Loading

Another thing to be aware of:
Laravel 10 supports MySQL 5.7 were the default collation is utf8mb4_general_ci see https://dev.mysql.com/doc/refman/5.7/en/charset-charsets.html

MySQL 5.7 also doesn't support the collation utf8mb4_0900_ai_ci and will reach end of support in October 2023. It is also not supported in Laravel 11 anymore. See https://laravel.com/docs/master/database#introduction

So setting the default for to utf8mb4_0900_ai_ci would only be possible in Laravel 11 for MySQL.

@binaryfire
Copy link
Contributor

binaryfire commented Sep 18, 2023 •
edited
Loading

Thanks for testing @Jubeki

I definitely think Laravel 11 should drop support for MySQL 5.x. There have been several other changes in v8, for example the password hashing algorithm. 8.0 was released in 2018 so it’s been out for 5 years now. And supporting backwards compatibility with 5.x would mean reduced compatibility with 8.0.

I still think separate mysql and mariadb connections make sense. Oracle are focused on making MySQL a hybrid NoSQL/SQL solution with features like the document store and a bunch of extra JSON functions. MariaDB is mainly focused on being a pure SQL db. I’m sure v9 will see an even bigger divergence between the two.

@binaryfire
Copy link
Contributor

binaryfire commented Sep 18, 2023 •
edited
Loading

Letting the db handle the default collation would be a good stopgap for v10 though. I just meant separate connections would be better long term.

@taylorotwell taylorotwell merged commit 74c5a01 into laravel:10.x Sep 19, 2023
@Jubeki Jubeki deleted the patch-2 branch September 19, 2023 14:24
@Jubeki Jubeki mentioned this pull request Sep 19, 2023
Merged
y-martini pushed a commit to webfucktory/laravel that referenced this pull request Sep 22, 2023
@Jubeki
Let database handle default collation (laravel#6241)
7cc5312
@thor thor mentioned this pull request Oct 8, 2023
Merged
This was referenced Nov 2, 2023
Merged
Closed
taylorotwell added a commit that referenced this pull request Nov 2, 2023
@taylorotwell
Revert "Let database handle default collation (#6241)"
d0bcb31 
This reverts commit 74c5a01.
taylorotwell added a commit that referenced this pull request Nov 2, 2023
@taylorotwell
Revert "Let database handle default collation (#6241)" (#6266)
024c86a 
This reverts commit 74c5a01.
@driesvints
Copy link
Member

@Jubeki we reverted this one

christopheredjohnson pushed a commit to TheYGSGroup/laravel that referenced this pull request Jan 5, 2024
@Jubeki
Let database handle default collation (laravel#6241)
54cfe8e
christopheredjohnson pushed a commit to TheYGSGroup/laravel that referenced this pull request Jan 5, 2024
@taylorotwell
Revert "Let database handle default collation (laravel#6241)" (larave…
971cf12 
…l#6266)

This reverts commit 74c5a01.
@crynobone crynobone mentioned this pull request Mar 14, 2024
Closed
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.
4 participants
@Jubeki @binaryfire @driesvints @taylorotwell