In C4::AuthoritiesMarc::merge, we search all biblio records using an authority with Koha::Authorities->linked_biblionumbers(). This method uses simple_search_compat() with no results limit, even saying in comment : # if max_results is undefined, we will get all results https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/Authorities.pm#L80 With Zebra searchengin it is correct. But not with Elasticsearch, there is an hardcoded limit at 100 in case no limit is given : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L346 This means authorities links are wrong after a merge or an edit in case the authority is used in more than 100 biblio records. :(
I propose to fix by using by default the real server max given by Koha::SearchEngine::Elasticsearch::Search::max_result_window(). This will allow a huge limit nearly impossible to reach. See Bug 30882 to how increase this limit.
Created attachment 135573 [details] [review] Bug 30883: Authorities merge is limited to 100 biblio with Elasticsearch In C4::AuthoritiesMarc::merge, we search all biblio records using an authority with Koha::Authorities->linked_biblionumbers(). This method uses simple_search_compat() with no results limit, even saying in comment : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/Authorities.pm#L80 With Zebra searchengin it is correct. But not with Elasticsearch, there is an hardcoded limit at 100 in case no limit is given : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L346 This means authorities links are wrong after a merge or an edit in case the authority is used in more than 100 biblio records. :( I propose to fix by using by default the real server max given by Koha::SearchEngine::Elasticsearch::Search::max_result_window(). This will allow a huge limit nearly impossible to reach. See Bug 30882 to how increase this limit. Test plan : 1) Use Elasticsearch search engine 2) Use an authority id=111 linked to 200 biblio records 3) Perform a search 'an:111', you get 200 results 4) Create a new authority id=222 linked to 2 biblio records 5) Perform a search 'an:222', you get 2 results 6) Perform a merge of the two authorties, keeping id=222 7) Perform a search 'an:111' without patch you get 100 results with patch you get no results 8) Perform a search 'an:222' without patch you get 102 results with patch you get 202 results
BTW we should think of making a background job of this action
Created attachment 135699 [details] [review] Bug 30883: Authorities merge is limited to 100 biblio with Elasticsearch In C4::AuthoritiesMarc::merge, we search all biblio records using an authority with Koha::Authorities->linked_biblionumbers(). This method uses simple_search_compat() with no results limit, even saying in comment : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/Authorities.pm#L80 With Zebra searchengin it is correct. But not with Elasticsearch, there is an hardcoded limit at 100 in case no limit is given : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L346 This means authorities links are wrong after a merge or an edit in case the authority is used in more than 100 biblio records. :( I propose to fix by using by default the real server max given by Koha::SearchEngine::Elasticsearch::Search::max_result_window(). This will allow a huge limit nearly impossible to reach. See Bug 30882 to how increase this limit. Test plan : 1) Use Elasticsearch search engine 2) Use an authority id=111 linked to 200 biblio records 3) Perform a search 'an:111', you get 200 results 4) Create a new authority id=222 linked to 2 biblio records 5) Perform a search 'an:222', you get 2 results 6) Perform a merge of the two authorties, keeping id=222 7) Perform a search 'an:111' without patch you get 100 results with patch you get no results 8) Perform a search 'an:222' without patch you get 102 results with patch you get 202 results Signed-off-by: David Nind <david@davidnind.com>
I finally managed to test this and get things to work (using koha-testing-docker). Initially, both before and after applying the patch, my results were: 1. Merged authority record deleted, it no longer existed. 2. Searches were the same before and after, that is, there were no changes in the number of results. 3. None of the records that had the old term were updated with the new term. The issue with merged authority records not updating the bibliographic records was the system preference AuthorityMergeLimit- this is set to 50 in koha-testing-docker. I'm pretty sure there are easier ways to add a new term to multiple records, but this is what I did: 1. Enage Elasticsearch as the search engine and reindexed: koha-elasticsearch --rebuild -d -b -a kohadev 2. AuthorityMergeLimit: set to 50, changed to 1,000. 3. Added two new topical terms: - Term to merge ==> id = 1708 - Term to keep (merge) ==> id = 1709 4. Manually added "Term to keep (merge)" (1709) to two records ==> in authority search, search for "merge" shows as 2 records "Term to keep (merge)" and 0 for "Term to merge" 5. Created a MARC modification template (Tools > Catalog) with two actions: 3.1 Add new field(s) 650$a with value Term to merge 3.2 Update existing or add new field(s) 650$9 with value 1708 if field 650$a matches Term to merge 6. Created a report to get a list of record ids, exported as a spreadsheet. SQL = select * from biblio; 7. Set up and ran a batch record modification: - Record type: Biblios - Enter a list of record numbers: copy and paste in 200 record ids from the exported report - MARC modification template: choose the template that you created - Click "Contine" - Check a record to see if it will add a 650$a and 650$9 - Click "Modifiy selected records' ==> in authority search, search for "merge": . "Term to merge" shows as 200 records . "Term to keep (merge)" shows as 2 records
*** Bug 30522 has been marked as a duplicate of this bug. ***
Created attachment 135997 [details] [review] Bug 30883: Authorities merge is limited to 100 biblio with Elasticsearch In C4::AuthoritiesMarc::merge, we search all biblio records using an authority with Koha::Authorities->linked_biblionumbers(). This method uses simple_search_compat() with no results limit, even saying in comment : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/Authorities.pm#L80 With Zebra searchengin it is correct. But not with Elasticsearch, there is an hardcoded limit at 100 in case no limit is given : https://git.koha-community.org/Koha-community/Koha/src/commit/44d6528b566e5c16eba9b15f8aa24667293766c3/Koha/SearchEngine/Elasticsearch/Search.pm#L346 This means authorities links are wrong after a merge or an edit in case the authority is used in more than 100 biblio records. :( I propose to fix by using by default the real server max given by Koha::SearchEngine::Elasticsearch::Search::max_result_window(). This will allow a huge limit nearly impossible to reach. See Bug 30882 to how increase this limit. Test plan : 1) Use Elasticsearch search engine 2) Use an authority id=111 linked to 200 biblio records 3) Perform a search 'an:111', you get 200 results 4) Create a new authority id=222 linked to 2 biblio records 5) Perform a search 'an:222', you get 2 results 6) Perform a merge of the two authorties, keeping id=222 7) Perform a search 'an:111' without patch you get 100 results with patch you get no results 8) Perform a search 'an:222' without patch you get 102 results with patch you get 202 results Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This looks great, and the QA scripts are happy. One minor though is should we have a unit test for this as it affects a module.. but I wasn't entirely sure how best to approach that so thought PQA and leave it for the RM to decide whether to require one. PQA
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.01
Pushed to 21.11.x for 21.11.10, thx!
Can't reproduce on 21.05.x 7) Perform a search 'an:111' without patch you get 100 results with patch you get no results 8) Perform a search 'an:222' without patch you get 102 results with patch you get 202 results With and without the patch, I always get the original number of results. Even if the 1st auth doesn't exist anymore. Note: I used preexisting authorities on sample data. respectively 1 (used in 152 records) and 101 (used in 4 records) Any idea? Does this show merge is completely broken in 21.05.x ? Maybe I used it badly. Anyway, the code very simple and safe so I don't think it's related: Backported: Pushed to 21.05.x branch for 21.05.17 Nothing to document, marking resolved.