1194221 - Change the reference data varchar column collations to utf8_bin instead of utf8_ci

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Mauro Doglio [:mdoglio]]

We don't need case sensitive filtering/sorting and using utf8_ci gives you better performance in query execution.

Comment 1

[Mauro Doglio [:mdoglio]]

Also, the django orm doesn't support non-default collation type on varchar/text fields.

Comment 2

[Mauro Doglio [:mdoglio]]

Given that the target tables are quite small we probably don't need to do anything fancy other than an alter table. :sheeri what do you think about it? The tables I'm talking about are those ones defined here https://github.com/mozilla/treeherder/blob/cc52bf7f35077ec529e35e0236948fc2a1f89c29/treeherder/model/sql/template_schema/treeherder_reference_1.sql.tmpl

Comment 3

[Sheeri Cabral [:sheeri]]

Ed Morley specifically used utf8_bin in https://bugzilla.mozilla.org/show_bug.cgi?id=1185030#c2

If the tables are small, does the performance difference matter? I ask b/c I assume in the bug Ed had good reason to change to a case-sensitive collation.

But yeah, we can change utf8_bin in the tables other than the datasource table (and anything else that uses tokens).

Comment 4

[Ed Morley [:emorley]]

(In reply to Sheeri Cabral [:sheeri] from comment #3)
> Ed Morley specifically used utf8_bin in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1185030#c2

That was not changing the collation, but setting a unique key, using the same collation as was there previously - so don't let it affect the decision here :-)

Comment 5

[Mauro Doglio [:mdoglio]]

ok I'll go ahead and change the sql template files we have in our repository. I guess I can produce a sql script to apply the change on staging and I'll coordinate with sheeri to do the same on production if that goes well.

Comment 6

[Sheeri Cabral [:sheeri]]

OK, sounds like a plan.

Comment 7

[Mauro Doglio [:mdoglio]]

Attachment: treeherder_alter_collation.sql

I tried to run this on staging. I put some notes in the file indicating the failures I got.

Comment 8

[Mauro Doglio [:mdoglio]]

I'm putting this work on hold until we have a way to clean the reference data tables.

Comment 9

[Mauro Doglio [:mdoglio]]

Since the final goal is to have a sql schema consistent with what the django orm would produce, I'm going to do it the other way around: utf8_bin everywhere! It should be sufficient to:
1- change the default db collation to utf8_bin
2- change the collation of those few utf8_unicode_ci tables to utf8_bin

Comment 10

[Ed Morley [:emorley]]

Is this something we will undo in the future?

Comment 11

[Ed Morley [:emorley]]

(In reply to Mauro Doglio [:mdoglio] from comment #9)
> Since the final goal is to have a sql schema consistent with what the django
> orm would produce, I'm going to do it the other way around: utf8_bin
> everywhere! It should be sufficient to:
> 1- change the default db collation to utf8_bin
> 2- change the collation of those few utf8_unicode_ci tables to utf8_bin

Updating the summary to reflect that.

Comment 12

[Sheeri Cabral [:sheeri]]

So, the one note I see that specifically talk about an error is:
/*altering the reference_data_signatures table raise errors for unique key constraints*/
alter table `reference_data_signatures` modify `name` varchar(255) character set utf8 collate utf8_general_ci;
...

When moving from utf8_bin to utf8_general_ci, ci = case insensitive, so if one value is "foo" and another "FOO" that can cause this type of error.

Comment 11 shows that you want to change the case insensitive stuff to binary (which is case sensitive) so this shouldn't be a problem.

Let me know if there's anything else I can help with. default charset is set to utf8 but there's no collation set, which defaults to utf8_general_ci. Do you want me to change the default collation on treeherder_stage and treeherder? Default collation is what new tables created will get.

Comment 13

[Mauro Doglio [:mdoglio]]

:sheeri yes, that would be great.

Comment 14

[Sheeri Cabral [:sheeri]]

OK, changing server default in bug 1201087. If you want me to change db defaults, let me know.