Skip to content

Set 'utf8mb4' when exporting without '--default-character-set' option #199

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

Merged

Conversation

ryotsun
Copy link
Contributor

@ryotsun ryotsun commented Jun 27, 2021

Fixes: #144

I'm not sure whether this approach is correct.
Kindly check this PR in your time.

@ryotsun ryotsun requested a review from a team as a code owner June 27, 2021 13:33
@schlessera schlessera added the bug label Jul 20, 2021
@schlessera schlessera added this to the 2.0.14 milestone Jul 20, 2021
@wojsmol
Copy link
Contributor

wojsmol commented Jul 20, 2021

@schlessera What if the database is in utf8 and by setting utf8mb4 as default same backup script will be broken?

@schlessera schlessera changed the title #144 set 'utf8mb4' when exports without '--default-charcter-set' option Set 'utf8mb4' when exporting without '--default-character-set' option Jul 20, 2021
@schlessera
Copy link
Member

@wojsmol Good question. As far as I know, utf8mb4 is fully backwards compatible with utf8, so I assume any utf8 export should just work to be imported into utf8mb4. Any potential data loss would already have happened before the export, as utf8 would have already stripped any 4-byte code points.

But I'm not an expert in DB encodings. Am I maybe missing something here?

@wojsmol
Copy link
Contributor

wojsmol commented Jul 20, 2021

Regarding characters maybe, but for shore there are deferences in length of the indexes - see this make/core blog post linked in the original issue.

@schlessera
Copy link
Member

@wojsmol I now added a detection of the actual character set of the wp_posts table. I think this should be safe enough now to use as a default, and reduce the risk of people actually losing their emojis on migration.

What do you think about that approach?

@schlessera schlessera merged commit 9b08c7d into wp-cli:master Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emoji like 🎥 will be changed to ? after wp db import
3 participants