There is a perl related bug in C4/Matcher.pm when collecting matched records. "simple_search_compat" returns a reference to an array of matches. When using ElasticSearch, this array contains MARC::Record objects. The score for these matches are then collected (currently line 692-694) into a hash "%matches" where the matched object is used as the key. Perl converts all hash keys into strings, so the object is lost. The hash "%matches" is used in a foreach (line 708) where it assumes the object can be recovered from the key, but this is not the case. The error in the log is: stage-marc-import.pl: Can't locate object method "fields" via package "MARC::Record=HASH(0xb403da8)" (perhaps you forgot to load "MARC::Record=HASH(0xb403da8)"?) at /home/vagrant/kohaclone/C4/Biblio.pm line 2691., referer: http://localhost:8081/cgi-bin/koha/tools/stage-marc-import.pl Steps to reproduce (given an instance using ElasticSearch as search engine): 1. Export a bibliographic record in MARC format. 2. Create a matching rule that matches on "999$c" with a score and threshold of 1000 (doesn't really matter as long as they're the same) 3. Edit the record in Koha so that they're slightly different (this is probably unnecessary). 4. Upload the exported file in Stage MARC import, and select the matching rule you created, then click on "Stage for import". Expected outcome: Redirected to the batch page with the matching record being shown. Actual outcome: Things seem to hang, but in reality the staging has crashed with the error shown above.
Created attachment 60716 [details] [review] Bug 18131: ES - Fix matching staged records The code in C4::Matches::get_matches is terrible and a bug has been introduced by bug 12478 because of its way to handle uniqueness. If search engine is elastic, simple_search_compat returns array ref of MARC::Record, used as a string for the key of the matches hashref we get things like "MARC::Record=HASH(0x8f76ab0)". Yes, terrible... The file is never staged and we get an internal server error: stage-marc-import.pl: Can't locate object method "fields" via package "MARC::Record=HASH(0x8f76ab0)" (perhaps you forgot to load "MARC::Record=HASH(0x8f76ab0)"?) at /home/vagrant/kohaclone/C4/Biblio.pm line 2691 To recreate the issue: - Set SearchEngine == Elastic - Create a matching rule on 999$c (you need to edit the existing one and specify 'Local-number' as search index, not 'local-number') - Import a file with bibliographic records and use the matching rule you defined. Test plan: Import authority and bibliographic records with Zebra and Elastic using a matching rule. Everything should work correctly. Note: I found a bug when importing authorities using Elastic, see bug 17255 comment 38.
Everything worked well for bibliogrpahic records, however, when I tried using an authority match rule I got: Can't locate object method "field" via package "<?xml version="1.0" encoding="UTF-8"?> <record xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.loc.gov/MARC21/slim" xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"> My rule was Local-number Field: 001
(In reply to Nick Clemens from comment #2) > Everything worked well for bibliogrpahic records, however, when I tried > using an authority match rule I got: Yes but, as you might know, the authority searches are broken all around Koha. Occurrences of C4::AuthoritiesMarc::SearchAuthorities must be replaced by search_auth_compat. I would consider this outside the scope of this bug report...
Created attachment 61586 [details] [review] Bug 18131: ES - Fix matching staged records - Authority Occurrences of C4::AuthoritiesMarc::SearchAuthorities must be replaced by search_auth_compat. You need to define the search index of matching rule with one of the values defined in %koha_to_index_name (from Koha::SearchEngine::Elasticsearch::QueryBuilder::build_authorities_query_compat)
Nick, this patch may fix your issue.
*** Bug 17546 has been marked as a duplicate of this bug. ***
testing again, this does remove the fields error from authorities, but things are not fixed on authority side: Matching rules can only use a small set of indexes in build_authorities_query_compat: Options are: mainmainentry, mainentry, match, match-heading, see-from, and thesaurus. Tag in Matchpoint components seems to be useless? Sorting is also broken and I had to force no sorting to get match results, and then they didn't match anything Options are: mainmainentry, mainentry, match, match-heading, see-from, and 373 thesaurus. As Joubu said in comment #3 this is all outside the scope of this bug report. Signing off on patches as they offer great improvement and fix matching for biblios - authorities issues should be filed as a new ticket
Created attachment 62087 [details] [review] Bug 18131: ES - Fix matching staged records The code in C4::Matches::get_matches is terrible and a bug has been introduced by bug 12478 because of its way to handle uniqueness. If search engine is elastic, simple_search_compat returns array ref of MARC::Record, used as a string for the key of the matches hashref we get things like "MARC::Record=HASH(0x8f76ab0)". Yes, terrible... The file is never staged and we get an internal server error: stage-marc-import.pl: Can't locate object method "fields" via package "MARC::Record=HASH(0x8f76ab0)" (perhaps you forgot to load "MARC::Record=HASH(0x8f76ab0)"?) at /home/vagrant/kohaclone/C4/Biblio.pm line 2691 To recreate the issue: - Set SearchEngine == Elastic - Create a matching rule on 999$c (you need to edit the existing one and specify 'Local-number' as search index, not 'local-number') - Import a file with bibliographic records and use the matching rule you defined. Test plan: Import authority and bibliographic records with Zebra and Elastic using a matching rule. Everything should work correctly. Note: I found a bug when importing authorities using Elastic, see bug 17255 comment 38. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 62088 [details] [review] Bug 18131: ES - Fix matching staged records - Authority Occurrences of C4::AuthoritiesMarc::SearchAuthorities must be replaced by search_auth_compat. You need to define the search index of matching rule with one of the values defined in %koha_to_index_name (from Koha::SearchEngine::Elasticsearch::QueryBuilder::build_authorities_query_compat) Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment on attachment 62087 [details] [review] Bug 18131: ES - Fix matching staged records Review of attachment 62087 [details] [review]: ----------------------------------------------------------------- ::: C4/Matcher.pm @@ +673,5 @@ > + else { > + if ( C4::Context->preference('SearchEngine') eq 'Elasticsearch' ) { > + foreach my $matched ( @{$searchresults} ) { > + my ( $biblionumber_tag, $biblionumber_subfield ) = C4::Biblio::GetMarcFromKohaField( "biblio.biblionumber", $marcframework_used ); > + my $id = $matched->field($biblionumber_tag)->subfield($biblionumber_subfield); This fails on a UNIMARC setup where $biblionumber_tag is 001 (should use ->data())
Also, this is weird that search_result_compat returns different result depending on the search engine. Shouldn't we fix one of Koha::SearchEngine::*::Search::search_result_compat ? Failing QA for comment 10
Created attachment 68884 [details] [review] Bug 18131: Followup - Use data if biblionumber_tag < 10
(In reply to Julian Maurice from comment #11) > Also, this is weird that search_result_compat returns different result > depending on the search engine. Shouldn't we fix one of > Koha::SearchEngine::*::Search::search_result_compat ? Agreed, it is odd, see the comment in the pod: 315 an arrayref of MARC::Records (note that this is different from the 316 L<C4::Search> version which will return plain XML, but too bad.) I think beyond the scope here, let's get this fixed for now.
Created attachment 69087 [details] [review] Bug 18131: ES - Fix matching staged records The code in C4::Matches::get_matches is terrible and a bug has been introduced by bug 12478 because of its way to handle uniqueness. If search engine is elastic, simple_search_compat returns array ref of MARC::Record, used as a string for the key of the matches hashref we get things like "MARC::Record=HASH(0x8f76ab0)". Yes, terrible... The file is never staged and we get an internal server error: stage-marc-import.pl: Can't locate object method "fields" via package "MARC::Record=HASH(0x8f76ab0)" (perhaps you forgot to load "MARC::Record=HASH(0x8f76ab0)"?) at /home/vagrant/kohaclone/C4/Biblio.pm line 2691 To recreate the issue: - Set SearchEngine == Elastic - Create a matching rule on 999$c (you need to edit the existing one and specify 'Local-number' as search index, not 'local-number') - Import a file with bibliographic records and use the matching rule you defined. Test plan: Import authority and bibliographic records with Zebra and Elastic using a matching rule. Everything should work correctly. Note: I found a bug when importing authorities using Elastic, see bug 17255 comment 38. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 69088 [details] [review] Bug 18131: ES - Fix matching staged records - Authority Occurrences of C4::AuthoritiesMarc::SearchAuthorities must be replaced by search_auth_compat. You need to define the search index of matching rule with one of the values defined in %koha_to_index_name (from Koha::SearchEngine::Elasticsearch::QueryBuilder::build_authorities_query_compat) Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 69089 [details] [review] Bug 18131: Followup - Use data if biblionumber_tag < 10 Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Pushed to master for 17.11, thanks to everybody involved!