SQL temp table not using utf8mb4 if server default already set to utf8mb4

[demeritcowboy]

Overview

A unit test was failing locally with row size too large but the db settings should have supported it. (CRM/Export/BAO/ExportTest.php)

Technical Details

One thing was createWithColumns was not using the getUtf8String() helper, so it was overriding the server default and putting utf8.

Another thing is that it wasn't clear what the intent of the if statement in that function was. If it was meant to be a boolean AND, then it would always fail. If it was meant to be a boolean OR, as I suspect, then why two clauses.

Comments

I don't know the exact server settings on the PR test sites but I'm a little confused as to why the test was working there before and not also getting row size too large. Maybe a different page_size or something?

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I think this is right. My head started hurting though.

[eileenmcnaughton]

OK I think what is hurting is the $this-utf8 part. Perhaps making it

  protected $utf8 = TRUE;

would be clearer - although I suspect the possibility of it being false is null & void now & it could just go.

Anyway this change makes sense

[demeritcowboy]

I agree the utf8 param can be removed, and it would seem reasonable to up the requirements to say you MUST use some variation of utf8 for your default db settings, since all the supported databases support that.

The setUtf8() function doesn't seem used in core and is actually marked deprecated just doesn't call the warning function, and as it notes in the comment if you really want a different charset/collation you can still do that on a per-column basis.

[eileenmcnaughton]

@demeritcowboy - #18133 - hit an issue with this when testing against our CI