Bug 19670 - search_marc_map.marc_field should have COLLATE= utf8mb4_bin
Summary: search_marc_map.marc_field should have COLLATE= utf8mb4_bin
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Ere Maijala
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-21 13:52 UTC by Nicolas Legrand
Modified: 2019-04-23 14:39 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.05


Attachments
Bug 19670: Change Collation of marc_field to allow mixed case mappings (2.15 KB, patch)
2019-03-14 12:34 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19670: Change Collation of marc_field to allow mixed case mappings (2.20 KB, patch)
2019-03-30 20:43 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 19670: Change Collation of marc_field to allow mixed case mappings (2.46 KB, patch)
2019-04-04 05:27 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19670: Change Collation of marc_field to allow mixed case mappings (2.46 KB, patch)
2019-04-08 21:39 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 19670: Change Collation of marc_field to allow mixed case mappings (2.52 KB, patch)
2019-04-09 09:17 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 2017-11-21 13:52:01 UTC
With COLLATE=utf8_unicode_ci it's not possible to map fields with same letter but different case (eg 995a and 995A) with the form of elasticsearch/mappings.pl. Changing the collation in the database fix the issue.
Comment 1 Katrin Fischer 2019-02-10 00:53:14 UTC
I confirm, it's still _ci:

--
-- Table structure for table 'search_marc_map'
--

DROP TABLE IF EXISTS search_marc_map;
CREATE TABLE `search_marc_map` (
  id int(11) NOT NULL AUTO_INCREMENT,
  index_name ENUM('biblios','authorities') NOT NULL COMMENT 'what storage index this map is for',
  marc_type ENUM('marc21', 'unimarc', 'normarc') NOT NULL COMMENT 'what MARC type this map is for',
  marc_field VARCHAR(255) NOT NULL COMMENT 'the MARC specifier for this field',
  PRIMARY KEY(`id`),
  UNIQUE key `index_name` (`index_name`, `marc_field` (191), `marc_type`),
  INDEX (`index_name`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

As we are also using uppercase letters to extent the item subfields, I'd be interested in fixing this. Note: we already have the different collation on 

marc_subfield_structure.tagsubfield

for the same reason.

Can we agree on this change?
Comment 2 Ere Maijala 2019-03-14 12:21:21 UTC
I'd say yes. Patch coming up..
Comment 3 Ere Maijala 2019-03-14 12:34:12 UTC
Created attachment 86599 [details] [review]
Bug 19670: Change Collation of marc_field to allow mixed case mappings

Test plan:
1. Apply patch and update database
2. Verify that you can add a search field mapping for both 100a and 100A.
3. Verify that the above also works with a newly-created database.
Comment 4 Michal Denar 2019-03-30 20:43:05 UTC
Created attachment 87240 [details] [review]
Bug 19670: Change Collation of marc_field to allow mixed case mappings

Test plan:
1. Apply patch and update database
2. Verify that you can add a search field mapping for both 100a and 100A.
3. Verify that the above also works with a newly-created database.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 5 Katrin Fischer 2019-04-03 15:54:38 UTC
As this is classified as a bugfix for backporting, I think we should add a check to the database update to make sure it's not run twice (Idempotent https://wiki.koha-community.org/wiki/Database_updates).

Could you please check?
Comment 7 Ere Maijala 2019-04-04 05:27:10 UTC
Created attachment 87379 [details] [review]
Bug 19670: Change Collation of marc_field to allow mixed case mappings

Test plan:
1. Apply patch and update database
2. Verify that you can add a search field mapping for both 100a and 100A.
3. Verify that the above also works with a newly-created database.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 8 Ere Maijala 2019-04-04 05:28:35 UTC
Requesting a new signoff since the check changes the patch so much.
Comment 9 Michal Denar 2019-04-08 21:39:51 UTC
Created attachment 87558 [details] [review]
Bug 19670: Change Collation of marc_field to allow mixed case mappings

Test plan:
1. Apply patch and update database
2. Verify that you can add a search field mapping for both 100a and 100A.
3. Verify that the above also works with a newly-created database.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 10 Katrin Fischer 2019-04-09 09:17:15 UTC
Created attachment 87590 [details] [review]
Bug 19670: Change Collation of marc_field to allow mixed case mappings

Test plan:
1. Apply patch and update database
2. Verify that you can add a search field mapping for both 100a and 100A.
3. Verify that the above also works with a newly-created database.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2019-04-09 09:17:27 UTC
Thanks, Ere!
Comment 12 Nick Clemens 2019-04-11 13:54:25 UTC
Awesome work all!

Pushed to master for 19.05
Comment 13 Martin Renvoize 2019-04-16 13:06:57 UTC
Pushed to 18.11.x for 18.11.05
Comment 14 Lucas Gass 2019-04-23 14:39:44 UTC
backported to 18.05.x for 18.05.12