Bug 23828 - Elasticsearch - ES - Authority record results not ordered correctly
Summary: Elasticsearch - ES - Authority record results not ordered correctly
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-16 15:28 UTC by Nick Clemens
Modified: 2020-11-17 20:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06


Attachments
Bug 23828: Unit tests (4.96 KB, patch)
2020-09-16 11:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings (3.25 KB, patch)
2020-09-16 11:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23828: Unit tests (4.87 KB, patch)
2020-09-22 12:32 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings (3.31 KB, patch)
2020-09-22 12:32 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23828: Unit tests (5.10 KB, patch)
2020-10-07 13:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings (3.31 KB, patch)
2020-10-07 13:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23828: Unit tests (4.81 KB, patch)
2020-10-15 22:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings (3.37 KB, patch)
2020-10-15 22:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23828: (QA follow-up) Fix number of tests (1008 bytes, patch)
2020-10-15 22:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings (3.44 KB, patch)
2020-10-22 07:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23828: Unit tests (5.25 KB, patch)
2020-10-22 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-10-16 15:28:07 UTC
For example:
Catholic Church Addresses, essays, lectures
Catholic Church El Salvador Addresses, essays, lectures
.
.
.
Catholic Church.


The 'El Salvador' displays first, but is sorted as coming after Address...
Catcholic hcurch with an ending period comes after Catholic Church with subfields

We should ideally generate the sort fields in the same order that fields are listed in the record - that seems to be how the display of results is formed
Comment 1 Myka Kennedy Stephens 2019-12-11 20:46:15 UTC
Part of this issue is resolved by tweaking the mappings for authorities in ES. The current community authority mappings are indexing MARC fields and subfields as strings. 
Example: 110(abcdefghjklnpqstvxyz) is mapped to Heading.
When "Catholic Church" is used for "Search main heading," the result list looks like this (subfields added for clarity):
- $a Catholic Church $x Clergy $v Biography
- $a Catholic Church $z Italy $x Clergy $v Biography
- $a Catholic Church $x Doctrines
- $a Catholic Church $x Fiction
- $a Catholic Church $v Pastoral letters and charges
- $a Catholic Church $x Adult Education $v Periodicals

It is ordering them this way because they are indexed with the subfields in alphabetical order -- despite the fact that the subfields do not appear in alphabetical order in the record.

When 110 is mapped to Heading, without the (abcd...), the result list looks like this:
- Catholic Church Adult Education Periodicals
- Catholic Church Adult Education United States
- Catholic Church Clergy Biography
- Catholic Church Doctrines
- Catholic Church Fiction
- Catholic Church Italy Clergy Biography

WHAT IS STILL A BUG >
If a heading has a period in it, it will still appear after all the headings without a period. In the Catholic Church example, some authority records use a period: 
Catholic Church. Archdiocese of San Salvador (El Salvador). Archbishop (1977-1980 : Romero)
That particular heading and all others that follow a similar format will not appear in the results list alphabetically. It appears at the end after all "Catholic Church" headings that do not use a period.
We should probably be escaping punctuation when searching and/or indexing authorities.
Comment 2 Nick Clemens 2020-09-16 11:54:30 UTC
Created attachment 110175 [details] [review]
Bug 23828: Unit tests
Comment 3 Nick Clemens 2020-09-16 11:54:35 UTC
Created attachment 110176 [details] [review]
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings

Currently if you combine subfields in the marc mappings the subfields are indexed in the order
listed in the mapping.

i.e. 650(avxyz) in mapping

and in record:
650 $aHeading $zGeosubdiv $vFormsubdiv

is indexed as:
Heading Formsubdiv Geosubdiv

We should preserve the order and index as:
Heading Geosubdiv Formsubdiv

We can use built in function in Marc::Field to achieve this

To test:
 1 - It is easy to find examples of this using authorities
 2 - Find or create a record with subfields order azv
     e.g. 150$aActresses$zUnited states$vBiography
 3 - Add or have a second authority
     e.g. 150$aActresses$vPortraits
 4 - Set an authorities mapping for 'Heading' to 150(abgvxyz)
     find at:
     Administration->Search engine configuration (Elasticsearch)->Authorities tab
 5 - Index the records in Elasticsearch
     perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1691 -ai 1692
 6 - View the first record in the ES index
     curl es:9200/koha_kohadev_authorities/data/1692?pretty
 7 - Note 'Heading' field is ordered as in the mapping
 8 - Search authorities for 'contains' "act"
 9 - Note the records sort incorrectly
10 - Apply patches
11 - perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1692
12 - curl es:9200/koha_kohadev_authorities/data/1692?pretty
13 - Note the order is now preserved
14 - Search authorities for 'contains' "act"
15 - Note the records sort correctly
Comment 4 Nick Clemens 2020-09-16 11:56:45 UTC
Note: these patches do not address punctuation, we should file a new bug for that issue
Comment 5 Nick Clemens 2020-09-16 14:54:47 UTC
Note: these patches do not address punctuation, we should file a new bug for that issue
Comment 6 Heather 2020-09-16 15:18:18 UTC
I put in Bug 26472 to address punctuation in general in the filing/ordering of authorities search results, having just come across a good example of the problem in our catalog!:)
Comment 7 Andrew Fuerste-Henry 2020-09-22 12:32:11 UTC
Created attachment 110547 [details] [review]
Bug 23828: Unit tests

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 8 Andrew Fuerste-Henry 2020-09-22 12:32:16 UTC
Created attachment 110548 [details] [review]
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings

Currently if you combine subfields in the marc mappings the subfields are indexed in the order
listed in the mapping.

i.e. 650(avxyz) in mapping

and in record:
650 $aHeading $zGeosubdiv $vFormsubdiv

is indexed as:
Heading Formsubdiv Geosubdiv

We should preserve the order and index as:
Heading Geosubdiv Formsubdiv

We can use built in function in Marc::Field to achieve this

To test:
 1 - It is easy to find examples of this using authorities
 2 - Find or create a record with subfields order azv
     e.g. 150$aActresses$zUnited states$vBiography
 3 - Add or have a second authority
     e.g. 150$aActresses$vPortraits
 4 - Set an authorities mapping for 'Heading' to 150(abgvxyz)
     find at:
     Administration->Search engine configuration (Elasticsearch)->Authorities tab
 5 - Index the records in Elasticsearch
     perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1691 -ai 1692
 6 - View the first record in the ES index
     curl es:9200/koha_kohadev_authorities/data/1692?pretty
 7 - Note 'Heading' field is ordered as in the mapping
 8 - Search authorities for 'contains' "act"
 9 - Note the records sort incorrectly
10 - Apply patches
11 - perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1692
12 - curl es:9200/koha_kohadev_authorities/data/1692?pretty
13 - Note the order is now preserved
14 - Search authorities for 'contains' "act"
15 - Note the records sort correctly

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 9 Katrin Fischer 2020-09-23 20:39:59 UTC
There are lots of merge markers in the test file - this hasn't applied cleanly when tested.
Comment 10 Nick Clemens 2020-10-07 13:18:19 UTC
Created attachment 111329 [details] [review]
Bug 23828: Unit tests

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>

Bug 23828: (follow-up) fix unit test merge
Comment 11 Nick Clemens 2020-10-07 13:18:22 UTC
Created attachment 111330 [details] [review]
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings

Currently if you combine subfields in the marc mappings the subfields are indexed in the order
listed in the mapping.

i.e. 650(avxyz) in mapping

and in record:
650 $aHeading $zGeosubdiv $vFormsubdiv

is indexed as:
Heading Formsubdiv Geosubdiv

We should preserve the order and index as:
Heading Geosubdiv Formsubdiv

We can use built in function in Marc::Field to achieve this

To test:
 1 - It is easy to find examples of this using authorities
 2 - Find or create a record with subfields order azv
     e.g. 150$aActresses$zUnited states$vBiography
 3 - Add or have a second authority
     e.g. 150$aActresses$vPortraits
 4 - Set an authorities mapping for 'Heading' to 150(abgvxyz)
     find at:
     Administration->Search engine configuration (Elasticsearch)->Authorities tab
 5 - Index the records in Elasticsearch
     perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1691 -ai 1692
 6 - View the first record in the ES index
     curl es:9200/koha_kohadev_authorities/data/1692?pretty
 7 - Note 'Heading' field is ordered as in the mapping
 8 - Search authorities for 'contains' "act"
 9 - Note the records sort incorrectly
10 - Apply patches
11 - perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1692
12 - curl es:9200/koha_kohadev_authorities/data/1692?pretty
13 - Note the order is now preserved
14 - Search authorities for 'contains' "act"
15 - Note the records sort correctly

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>
Comment 12 Nick Clemens 2020-10-07 13:19:04 UTC
(In reply to Katrin Fischer from comment #9)
> There are lots of merge markers in the test file - this hasn't applied
> cleanly when tested.

Only the unit tests failed, the testing should still be valid as the code itself applied and worked
Comment 13 Katrin Fischer 2020-10-15 22:45:37 UTC
Created attachment 111813 [details] [review]
Bug 23828: Unit tests

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>

Bug 23828: (follow-up) fix unit test merge

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-10-15 22:45:43 UTC
Created attachment 111814 [details] [review]
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings

Currently if you combine subfields in the marc mappings the subfields are indexed in the order
listed in the mapping.

i.e. 650(avxyz) in mapping

and in record:
650 $aHeading $zGeosubdiv $vFormsubdiv

is indexed as:
Heading Formsubdiv Geosubdiv

We should preserve the order and index as:
Heading Geosubdiv Formsubdiv

We can use built in function in Marc::Field to achieve this

To test:
 1 - It is easy to find examples of this using authorities
 2 - Find or create a record with subfields order azv
     e.g. 150$aActresses$zUnited states$vBiography
 3 - Add or have a second authority
     e.g. 150$aActresses$vPortraits
 4 - Set an authorities mapping for 'Heading' to 150(abgvxyz)
     find at:
     Administration->Search engine configuration (Elasticsearch)->Authorities tab
 5 - Index the records in Elasticsearch
     perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1691 -ai 1692
 6 - View the first record in the ES index
     curl es:9200/koha_kohadev_authorities/data/1692?pretty
 7 - Note 'Heading' field is ordered as in the mapping
 8 - Search authorities for 'contains' "act"
 9 - Note the records sort incorrectly
10 - Apply patches
11 - perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1692
12 - curl es:9200/koha_kohadev_authorities/data/1692?pretty
13 - Note the order is now preserved
14 - Search authorities for 'contains' "act"
15 - Note the records sort correctly

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2020-10-15 22:45:49 UTC
Created attachment 111815 [details] [review]
Bug 23828: (QA follow-up) Fix number of tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Jonathan Druart 2020-10-22 07:57:29 UTC
Created attachment 112142 [details] [review]
Bug 23828: Elasticsearch - Preserve record order when combining subfield in mappings

Currently if you combine subfields in the marc mappings the subfields are indexed in the order
listed in the mapping.

i.e. 650(avxyz) in mapping

and in record:
650 $aHeading $zGeosubdiv $vFormsubdiv

is indexed as:
Heading Formsubdiv Geosubdiv

We should preserve the order and index as:
Heading Geosubdiv Formsubdiv

We can use built in function in Marc::Field to achieve this

To test:
 1 - It is easy to find examples of this using authorities
 2 - Find or create a record with subfields order azv
     e.g. 150$aActresses$zUnited states$vBiography
 3 - Add or have a second authority
     e.g. 150$aActresses$vPortraits
 4 - Set an authorities mapping for 'Heading' to 150(abgvxyz)
     find at:
     Administration->Search engine configuration (Elasticsearch)->Authorities tab
 5 - Index the records in Elasticsearch
     perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1691 -ai 1692
 6 - View the first record in the ES index
     curl es:9200/koha_kohadev_authorities/data/1692?pretty
 7 - Note 'Heading' field is ordered as in the mapping
 8 - Search authorities for 'contains' "act"
 9 - Note the records sort incorrectly
10 - Apply patches
11 - perl misc/search_tools/rebuild_elaticsearch.pl -a -ai 1692
12 - curl es:9200/koha_kohadev_authorities/data/1692?pretty
13 - Note the order is now preserved
14 - Search authorities for 'contains' "act"
15 - Note the records sort correctly

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD amended patch: Fix
   FAIL   spelling
     combind ==> combined
Comment 17 Jonathan Druart 2020-10-22 07:59:02 UTC
Created attachment 112143 [details] [review]
Bug 23828: Unit tests

Signed-off-by: Heather Hernandez <heather_hernandez@nps.gov>

Bug 23828: (follow-up) fix unit test merge

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 23828: (QA follow-up) Fix number of tests

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Jonathan Druart 2020-10-22 08:24:55 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 19 Lucas Gass 2020-11-13 16:18:26 UTC
backported to 20.05.x for 20.05.06
Comment 20 Aleisha Amohia 2020-11-17 04:11:41 UTC
enhancement, not backported to 19.11.x