Skip to content

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

Merged
schlessera merged 7 commits into
wp-cli:masterfrom
ryotsun:fix/#144_change_default_character_set
Jul 26, 2021
Merged

Set 'utf8mb4' when exporting without '--default-character-set' option#199
schlessera merged 7 commits into
wp-cli:masterfrom
ryotsun:fix/#144_change_default_character_set

Conversation

@ryotsun

@ryotsun ryotsun commented Jun 27, 2021

Copy link
Copy Markdown
Contributor

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
Comment thread src/DB_Command.php Outdated
@schlessera schlessera added the bug label Jul 20, 2021
@schlessera schlessera added this to the 2.0.14 milestone Jul 20, 2021
Comment thread src/DB_Command.php Outdated
@wojsmol

wojsmol commented Jul 20, 2021

Copy link
Copy Markdown
Contributor

@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 Jul 20, 2021
@schlessera

Copy link
Copy Markdown
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

wojsmol commented Jul 20, 2021

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

3 participants