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.

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

!!!TASK: Change default charset and collation to utf8mb4 #1267

Merged
merged 6 commits into from Apr 19, 2018

Conversation

kdambekalns
Copy link
Member

@kdambekalns kdambekalns commented Apr 9, 2018

This changes the charset and collation to create table statements in the
existing migrations. This make sure the tables are set up correctly
independent of the database default configuration.

This is breaking if you have existing tables that do not use ut8mb4 as
charset and utf8mb4_unicode_ci as collation. To solve this you need to
convert the existing tables. This can be done using the command::

./flow database:setcharset

This will convert the database configured in the settings and all tables
inside to use a default character set of utf8mb4 and a default collation
of utf8mb4_unicode_ci. It will also convert all character type columns
to that combination of charset and collation.

Background information on why this is done can be found in
https://medium.com/@adamhooper/in-mysql-never-use-utf8-use-utf8mb4-11761243e434

This changes the charset and collation to create table statements in the
existing migrations. This make sure the tables are set up correctly
independent of the database default configuration.

This is breaking if you have existing tables that do not use ut8mb4 as
charset and utf8mb4_unicode_ci as collation. To solve this you need to
convert the existing tables. This can be done using the command::

  ./flow database:setcharset

This will convert the database configured in the settings and all tables
inside to use a default character set of utf8mb4 and a default collation
of utf8mb4_unicode_ci. It will also convert all character type columns
to that combination of charset and collation.

Background information on why this is done can be found in
https://medium.com/@adamhooper/in-mysql-never-use-utf8-use-utf8mb4-11761243e434
@albe
Copy link
Member

albe commented Apr 9, 2018

The nice thing is, that our tests directly show the single one drawback of using utf8mb4 😆
Specified key was too long; max key length is 767 bytes
Making a varchar[(255)] column with collation utf8mb4 an index, will cause this issue.

@kitsunet
Copy link
Member

kitsunet commented Apr 9, 2018

Yep, that's an issue... But we must face it! 😆

@kdambekalns kdambekalns moved this from Needs review to Needs more work in Neos 4.0 & Flow 5.0 Release Board Apr 11, 2018
@kdambekalns
Copy link
Member Author

See #1268 for all related PRs

@kdambekalns
Copy link
Member Author

Ok, digging into the key length issue… no problems locally. Running MariaDB 10.2.12, and for all I can find, this is solved as of MariaDB 10.2.2 (which was released in 2016-09-27). With MySQL one needs MySQL v5.7.7 (released 2015-04-08) for this to "magically" be solved.

If this is the case… we should probably just update the minimum DB versions, with PHP 7.1 being required and 2018 being a quarter in, that seems appropriate to me.

From https://laravel.com/docs/master/migrations#creating-indexes

If you are running a version of MySQL older than the 5.7.7 release or MariaDB older than the 10.2.2 release, you may need to manually configure the default string length generated by migrations in order for MySQL to create indexes for them.
[…]
Alternatively, you may enable the innodb_large_prefix option for your database.

@kdambekalns
Copy link
Member Author

Ok, with MariaDB 10.2 the "MySQL" tests do pass. PostgreSQL fails, but that is because Doctrine passes the charset from the (changed) default settings on, and PostgreSQL does not understand utf8mb4. Why should it, eh?

The question now is… do we build in some more magic to (re)set this depending on the DB used, allow to configure options per driver used or simply require users to change the settings as needed when using PostgreSQL (and adjust our test setup)?

@kdambekalns
Copy link
Member Author

Some more changes to this one… and with neos/BuildEssentials#34 PostgreSQL should work as well.

@kdambekalns kdambekalns moved this from Needs more work to Needs review in Neos 4.0 & Flow 5.0 Release Board Apr 17, 2018
@kitsunet
Copy link
Member

YAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY I like it, lets do that!

@kdambekalns
Copy link
Member Author

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Green and looking good by reading.

@kitsunet kitsunet merged commit 2078f91 into neos:master Apr 19, 2018
Neos 4.0 & Flow 5.0 Release Board automation moved this from Needs review to Done Apr 19, 2018
@kdambekalns kdambekalns deleted the use-utf8mb4-charset branch April 19, 2018 16:26
ComiR pushed a commit to ComiR/flow-development-collection that referenced this pull request Aug 17, 2018
!!!TASK: Change default charset and collation to utf8mb4

This changes the charset and collation to create table statements in the
existing migrations. This make sure the tables are set up correctly
independent of the database default configuration.

This is breaking if you have existing tables that do not use ut8mb4 as
charset and utf8mb4_unicode_ci as collation. To solve this you need to
convert the existing tables. This can be done using the command::

  ./flow database:setcharset

This will convert the database configured in the settings and all tables
inside to use a default character set of utf8mb4 and a default collation
of utf8mb4_unicode_ci. It will also convert all character type columns
to that combination of charset and collation.

Background information on why this is done can be found in
https://medium.com/@adamhooper/in-mysql-never-use-utf8-use-utf8mb4-11761243e434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants