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

dev/core#339 Install CiviCRM tables using utf8mb4 #18960

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

seamuslee001
Copy link
Contributor

Overview

This changes the standard civicrm mysql tables to be in utf8mb4 and associated charset

Before

Tables installed using utf8

After

Tables installed using utf8mb4

ping @eileenmcnaughton @totten @demeritcowboy

@civibot
Copy link

civibot bot commented Nov 12, 2020

(Standard links)

@civibot civibot bot added the master label Nov 12, 2020
@demeritcowboy
Copy link
Contributor

Cool. FYI these 4 tests should fail: #18828 (comment)

The last one I think I was mistaken and it's also just a test update needed.

@eileenmcnaughton
Copy link
Contributor

Ok - so to articulate this makes it so that new installs install in utf8mb4 & the arguments are

  1. converting is already supported so the fact more sites will be on utf8mb4 is not a material change
  2. there is some risk that some extensions will install in utf8 AND the relevant fields will be used in a join - in which case they are incompatible but the majorty of extensions are probably fine
  3. new installs should get the benefit of the new approach & not have to later update
  4. this is in effect upping the presure on our existing path
  5. we no longer support a level of mysql that does not support uft8mb4 - it's likely some sites still use one but unlikely for new installs

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton fixed the tests

@demeritcowboy
Copy link
Contributor

If you install sample data the custom field tables are still utf8 - from civicrm_generated.mysql. Otherwise looks good to me.

@seamuslee001
Copy link
Contributor Author

Thanks @demeritcowboy updated that now @eileenmcnaughton you good to MOP this?

@seamuslee001 seamuslee001 force-pushed the utf8mb4_install branch 2 times, most recently from d154f57 to 51e4235 Compare November 14, 2020 01:41
@seamuslee001
Copy link
Contributor Author

Jenkins retest this please

@demeritcowboy
Copy link
Contributor

@seamuslee001 that was the 4th test that failed for me earlier. Me thinks it's a real fail and it doesn't like COMPRESSED. I was surprised it didn't fail the first time here.

Update sample custom value tables as appropriate as well

Fix conflict between test setting ROW_FORMAT being compressed and the ROW_FORMAT of the original table being DYNAMIC
@seamuslee001
Copy link
Contributor Author

@demeritcowboy I think I have found it, I had updated install query to specify the ROW_FORMAT was to be DYNAMIC, the problem being is that that was being carried through to the log table even tho the test was specifying COMPRESSED which would have been ok other than the fact that the test was putting in a KEY_BLOCK_SIZE setting for the table which is incompatible with DYNAMIC row format as per https://dev.mysql.com/doc/refman/5.6/en/innodb-compression-syntax-warnings.html

@eileenmcnaughton
Copy link
Contributor

This was notified in the dev-digest. Tests now passing & we have discussed this - merging

@eileenmcnaughton eileenmcnaughton merged commit e60f1f4 into civicrm:master Nov 16, 2020
@eileenmcnaughton eileenmcnaughton deleted the utf8mb4_install branch November 16, 2020 06:01
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.

3 participants