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

Fix MakeFullUnicode and add more unicode columns. #886

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wolfmanstout
Copy link
Contributor

@wolfmanstout wolfmanstout commented Oct 4, 2024

Important notes:

  • I have tested that this repairs the table in the Docker image, but I can't easily test if this works in a fresh Docker build, because the Docker build is currently broken. I have every reason to believe it would work, unless something was depending on the undocumented old behavior of modifying all columns.
  • In theory, the new MakeFullUnicode lines should not be needed in a fresh Docker build, but they also shouldn't hurt (at some point you probably want to move everything to utf8mb4). They are needed to fix the results of an earlier run of this script (because the column charsets would be inconsistent).
  • In terms of performance, I'm not sure what exactly to test, and also it's important to remember that I'm running MySQL 8 (via the Docker image) which is known to have improved utf8mb4 performance. I can say that in my configuration, searches are plenty fast, and the MySQL docs suggest that in MySQL 8 it's a bug if utf8mb4 is ever slower than utf8mb3. But I don't know what MySQL 5.7 performance looks like. If you are concerned, we could remove the MakeFullUnicode lines (with the understanding that this script won't fix tables that are inconsistent, but it'll work with fresh builds).
  • I have tried to preserve what seemed to be the original intent of MakeFullUnicode here. I don't think the ALTER TABLE ... DEFAULT CHARACTER SET is strictly needed if the goal is simply to pinpoint a single column. Conversely, if you want to update every column in the table, you could drop the columnname argument, run the ALTER TABLE ... CONVERT TO CHARACTER SET command to set both charset and collation, and skip the column-specific query. This would automatically apply to all character columns in the table.

Fixes #885

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set the 4 name fields to utf8mb4
1 participant