Bug 13810 - Prevent updatedatabase.pl from breaking because of case insensitive utf8 collation
Summary: Prevent updatedatabase.pl from breaking because of case insensitive utf8 coll...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 11944
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-09 16:06 UTC by Nicolas Legrand
Modified: 2015-12-03 22:11 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Change default collation to utf8_bin (1.72 KB, patch)
2015-03-09 16:17 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Reproduce Bug 13810 (1.67 KB, text/plain)
2015-04-02 13:23 UTC, Nicolas Legrand
Details
Bug 13810: Change collate for tagsubfield (utf8_bin) (3.49 KB, patch)
2015-04-27 10:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13810: Change collate for tagsubfield (utf8_bin) (4.10 KB, patch)
2015-04-27 13:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13810: Change collate for tagsubfield (utf8_bin) (4.16 KB, patch)
2015-04-29 12:41 UTC, Nicolas Legrand
Details | Diff | Splinter Review
[PASSED QA] Bug 13810: Change collate for tagsubfield (utf8_bin) (4.22 KB, patch)
2015-05-03 22:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Legrand 2015-03-09 16:06:28 UTC
When upgrading database from 3.18 to master upgrade Upgrade to 3.19.00.006 (Bug 11944 - Convert DB tables to utf8_unicode_ci) may fail like this:

DBD::mysql::db do failed: Duplicate entry 'ACQ-995-a' for key 'PRIMARY' at ./installer/data/mysql/updatedatabase.pl line 9671.

This is from table marc_subfield_structure where the keys are made of framework (ACQ), tagfield (995) and tagsubfield (a). If we have a key ACQ-995-A it thinks the entries are duplicates, because the collation is case insensitive. It's rather annoying, since it's legit in mark to have a subfield 'a' different from subfield 'A' inside the same subfield.

I propose to default collation on one which is not case insensitive, like utf8_bin.
Comment 1 Nicolas Legrand 2015-03-09 16:17:35 UTC Comment hidden (obsolete)
Comment 2 Nicolas Legrand 2015-03-11 15:31:35 UTC
putting utf8_bin in place of everything else breaks something. The intranet launches the web installer instead of the login prompt.
Comment 3 Katrin Fischer 2015-03-25 13:49:25 UTC
Quite worried about this one as we are also using upper case subfields in some cases (running out of item subfields). Is there a way to fix this?
Comment 4 Nicolas Legrand 2015-03-26 09:22:18 UTC
I'm not on it right now, because I find a way to have a working master branch. Not sure if it has everything it should about collation though.

I tried to debug this upgrading step by step, commit to updatedatabase.pl by commit. It appears it ran smoothly without complains. I realised I launched a different version of the dbrev 3.19.00.006: the one from commit 42a8082c8ffd05b3719b79060d81ba77d4f508d3. I then thought I was having trouble with the code added by Martin from commit a303cdec4871d3d3740d419c972eea7faf8e5a8a.

I'm not sure it make sense. But I didn't have complains when upgrading and still have my items attached to biblios in intranet (I was losing them when first trying with code from commit 
a303cdec4871d3d3740d419c972eea7faf8e5a8a).
Comment 5 Nicolas Legrand 2015-04-02 13:23:34 UTC
Created attachment 37437 [details]
Reproduce Bug 13810

This patch create subfields that should break when updating DBRev to 3.19.00.006. Here is the ouptut on a 3.18.x box :

    ./reproduce_bug_13810.pl
    Add subfields 666z and 666Z
    Try update to DBRev 3.19.00.006 for marc_subfield_structure only
    DBD::mysql::db do failed: Duplicate entry '666-z' for key 'PRIMARY' at ./reproduce_bug_13810.pl line 29.
    Remove subfields 666z and 666Z
Comment 6 Nicolas Legrand 2015-04-02 14:37:53 UTC
also, I was wrong, it seems commit a303cdec4871d3d3740d419c972eea7faf8e5a8a does also break everything
Comment 7 Jonathan Druart 2015-04-27 10:48:30 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-04-27 10:52:17 UTC
Nicolas,
If you have a look at a 3.18.x branch, you will see that the marc_subfield_structure.tagsubfield had a specific collate.
We should keep utf8_unicode_ci for other tables and columns, so I suggest to set back utf8_bin for this column.
Comment 9 Jonathan Druart 2015-04-27 10:52:48 UTC
Please obsolete your patch if you agree with mine.
Comment 10 Jonathan Druart 2015-04-27 10:54:10 UTC
This issue could cause failures when upgrading from 3.18 to 3.20, I am raising the priority.
It should be fixed before the 3.20 release.
Comment 11 Jonathan Druart 2015-04-27 13:50:20 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2015-04-28 08:37:13 UTC
I agree - updating the priority some more.
Comment 13 Nicolas Legrand 2015-04-29 11:48:31 UTC
Comment on attachment 36744 [details] [review]
Change default collation to utf8_bin

Jonathan's patch seems far better.
Comment 14 Nicolas Legrand 2015-04-29 12:41:26 UTC Comment hidden (obsolete)
Comment 15 Nicolas Legrand 2015-04-29 12:43:13 UTC
Note I've only done test plan 1, I'm working on a base that was breaking, I don't have the one to try to break :). Thanks a lot for the patch !
Comment 16 Katrin Fischer 2015-04-30 06:43:28 UTC
A first thought: Should we also take care of the auth_subfield_structure table?
Comment 17 Jonathan Druart 2015-04-30 07:25:57 UTC
(In reply to Katrin Fischer from comment #16)
> A first thought: Should we also take care of the auth_subfield_structure
> table?

The only place where the collate utf8_bin was used before bug 11944 was on marc_subfield_structure.tagsubfield.
If we want to change it for auth_subfield_structure.tagsubfield it won't be a bugfix but an enh and should be done in another bug report.
Comment 18 Katrin Fischer 2015-04-30 07:34:10 UTC
Hi Jonathan, cool - I didn't check. This should bring us back to the before-state then.
Comment 19 Katrin Fischer 2015-05-03 22:01:36 UTC
Created attachment 38793 [details] [review]
[PASSED QA] Bug 13810: Change collate for tagsubfield (utf8_bin)

Before bug 11944, the marc_subfield_structure.tagsubfield column had a
specific collate: utf8_bin. It has been introduced by
  commit 67e20d82ffdfcd69344ec30696bebc51c00d863c
  Date:   Sun Jun 22 16:35:48 2008 -0500
      DB Bump 094 - bug 2268 -- allow mixed case subfield labels in MARC21
      by changing db column collation.

This change should be kept and Koha should continue to allow and create
subfields with the same letter but different case sensitivity.

What does this patch:
1/ To prevent the updatedatabase entry 3.19.00.006 to fail if subfields
with different case sensitivity already exist in the DB, the table is
managed separately from others.
2/ To update DB which have already pass this dbrev, a new entry will be
create to update the specific collate for this column.

Test plan:
1/ a. With a 3.18 DB, create subfield 'a' and 'A' for the same field
   b. Execute the updatedatabase.pl script. 3.19.00.006 should not fail
      anymore
2/ a. With a master DB (3.19.00.006 has already been executed), create
subfields 'a' and 'A'. You should get an error.
   b. Apply this patch, execute the DB entry and try again 2/a. you
   should be able to create the second subfield.

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Tomás Cohen Arazi 2015-05-04 14:48:26 UTC
Fix pushed to master.

Thanks Jonathan!