|
Description
Nick Clemens (kidclamp)
2017-11-09 16:23:43 UTC
Created attachment 69072 [details] [review] Bug 19604: Elasticsearch Fixes for build_authorities_query for auth searching To test: 1 - Do some authority searches in Zebra 2 - Switch to ES and repeat, results will vary and some may fail 3 - Apply patch and dependencies 4 - Reindex ES 5 - Repeat searches, they should suceed and results should be similar to Zebra 6 - Slight differences are okay, but results should (mostly) meet expectations A few notes: We add a 'normalizer' to ensure we get a single token from the heading indexes, this makes 'starts with' work as expcted We switch to 'AND' for fields searched from cataloging editor - this matches Zebra results We force the '__sort' fields for sorting - if sorting looks wrong try reducing the heading field to a single subfield - this will need to be addressed on a future bug (multiple subfields create an array, ES sorts those randomly) I did a search for 'me' from the OPAC's Auth search. With Zebra I got 1411 results (some, but not all, matching on 'mé') With ES I got a single result, in which the word 'Me' was found as-is Created attachment 69198 [details] [review] Bug 19604: Elasticsearch Fixes for build_authorities_query for auth searching To test: 1 - Do some authority searches in Zebra 2 - Switch to ES and repeat, results will vary and some may fail 3 - Apply patch and dependencies 4 - Reindex ES 5 - Repeat searches, they should suceed and results should be similar to Zebra 6 - Slight differences are okay, but results should (mostly) meet expectations A few notes: We add a 'normalizer' to ensure we get a single token from the heading indexes, this makes 'starts with' work as expcted We switch to 'AND' for fields searched from cataloging editor - this matches Zebra results We force the '__sort' fields for sorting - if sorting looks wrong try reducing the heading field to a single subfield - this will need to be addressed on a future bug (multiple subfields create an array, ES sorts those randomly) Created attachment 69199 [details] [review] Bug 19604: Unit tests 1 - Apply previous patches 2 - prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t 3 - Should be green 4 - high fives! (In reply to David Bourgault from comment #2) > I did a search for 'me' from the OPAC's Auth search. > > With Zebra I got 1411 results (some, but not all, matching on 'mé') > With ES I got a single result, in which the word 'Me' was found as-is I altered the 'contains' search to be a wildcard and break terms - it should be closer now but won't mirror exactly zebra Tests passed, QA tools green. However, whereas I had 1 exact 1, I now have 0 search results. I tried reseting the mappings, I reindexed -d -a -v twice but I can't get it to find results. Testing in Kibana with the query `Heading:*me*`, I got multiple results (including both 'me' and 'mé') so I don't think it's a problem specific to my index/database. Failed QA :( (In reply to David Bourgault from comment #6) > Tests passed, QA tools green. > > However, whereas I had 1 exact 1, I now have 0 search results. > > I tried reseting the mappings, I reindexed -d -a -v twice but I can't get it > to find results. > > Testing in Kibana with the query `Heading:*me*`, I got multiple results > (including both 'me' and 'mé') so I don't think it's a problem specific to > my index/database. > > Failed QA :( Can you let me know the exact search and terms you are using? and the data in your DB? with patches applied and default data from kohadevbox I get results for 'me' or 'mé' and all are as expected Still applies - queries return well for me, moving back to needs signoff Created attachment 72807 [details] [review] Bug 19604: Elasticsearch Fixes for build_authorities_query for auth searching To test: 1 - Do some authority searches in Zebra 2 - Switch to ES and repeat, results will vary and some may fail 3 - Apply patch and dependencies 4 - Reindex ES 5 - Repeat searches, they should suceed and results should be similar to Zebra 6 - Slight differences are okay, but results should (mostly) meet expectations A few notes: We add a 'normalizer' to ensure we get a single token from the heading indexes, this makes 'starts with' work as expcted We switch to 'AND' for fields searched from cataloging editor - this matches Zebra results We force the '__sort' fields for sorting - if sorting looks wrong try reducing the heading field to a single subfield - this will need to be addressed on a future bug (multiple subfields create an array, ES sorts those randomly) Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Created attachment 72808 [details] [review] Bug 19604: Unit tests 1 - Apply previous patches 2 - prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t 3 - Should be green 4 - high fives! Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Test plans worked as it should with unimarc. Biggest difference I noticed, שלו (shalev) matches only שלו with unpatched ES and שלו but also שלום (shalom) with Zebra and with ES patched. And also sorting. Latin only authorities tend to be first with ES. Ended here trying to fix the Internal server error I have with every authority search (OPAC, Authorities module and linker plugin). Error is: [Request] ** [http://localhost:9200]-[400] [query_shard_exception] No mapping found for [Heading__sort.phrase] in order to sort on, with: {"index":"koha_kohadev_authorities","index_uuid":"FwdgBXbyRq-5E0F3sa4LTA"}, called from sub Search::Elasticsearch::Role::Client::Direct::__ANON__ at /usr/share/perl5/Catmandu/Store/ElasticSearch/Bag.pm line 167. With vars: {'status_code' => 400,'body' => {'error' => {'root_cause' => [{'type' => 'query_shard_exception','index' => 'koha_kohadev_authorities','index_uuid' => 'FwdgBXbyRq-5E0F3sa4LTA','reason' => 'No mapping found for [Heading__sort.phrase] in order to sort on'}],'grouped' => bless( do{\(my $o = 1)}, 'JSON::PP::Boolean' ),'reason' => 'all shards failed','phase' => 'query','failed_shards' => [{'shard' => 0,'node' => 'vSTPqetISteesIkQSA_Ehg','reason' => {'index' => 'koha_kohadev_authorities','reason' => 'No mapping found for [Heading__sort.phrase] in order to sort on','index_uuid' => 'FwdgBXbyRq-5E0F3sa4LTA','type' => 'query_shard_exception'},'index' => 'koha_kohadev_authorities'}],'type' => 'search_phase_execution_exception'},'status' => 400},'request' => {'ignore' => [],'method' => 'GET','mime_type' => 'application/json','qs' => {},'path' => '/koha_kohadev_authorities/data/_search','serialize' => 'std','body' => {'size' => 20,'sort' => [{'Heading__sort.phrase' => 'asc'}],'query' => {'bool' => {'must' => [{'wildcard' => {'Heading-Main.phrase' => '*der*'}}]}},'from' => 0}}} Unable to perform your search. Please try again. Can you help? Tried a full reindex, bibliographic search works. Katrin, I have some related improvements in bug 19365, but I'm waiting for this to get pushed first before rebasing them. If this and that's not enough, I should have something more to contribute since we have them working quite nicely locally. Hi, I not obtain the same result as zebra with applying this patch. I try the simple search : test using operator contain in zebra i have : 4 results In ES, I have : 2 results without this patch, I obtain : In zebra i have : 4 results In ES, I have : 1 result Thanks. Bouzid. (In reply to Bouzid Fergani from comment #15) > Hi, > > I not obtain the same result as zebra with applying this patch. > I try the simple search : test using operator contain > in zebra i have : 4 results > In ES, I have : 2 results > without this patch, I obtain : > In zebra i have : 4 results > In ES, I have : 1 result > Thanks. > Bouzid. Hi Bouzid, it's hard to tell, if that's right or wrong from your description.. We can't always expect the same results. What was your search? On which kind of data? Which results where missing you had expected to be included? Patch doesn't apply Should conflict with bug 20073 Basic searches for authorities with a sort not being 'none' fail and block other bugs. Patches here currently don't apply, can someone take a look? Created attachment 76977 [details] [review] Bug 19604: Elasticsearch Fixes for build_authorities_query for auth searching To test: 1 - Do some authority searches in Zebra 2 - Switch to ES and repeat, results will vary and some may fail 3 - Apply patch and dependencies 4 - Reindex ES 5 - Repeat searches, they should suceed and results should be similar to Zebra 6 - Slight differences are okay, but results should (mostly) meet expectations A few notes: We add a 'normalizer' to ensure we get a single token from the heading indexes, this makes 'starts with' work as expcted We switch to 'AND' for fields searched from cataloging editor - this matches Zebra results We force the '__sort' fields for sorting - if sorting looks wrong try reducing the heading field to a single subfield - this will need to be addressed on a future bug (multiple subfields create an array, ES sorts those randomly) Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Created attachment 76978 [details] [review] Bug 19604: Unit tests 1 - Apply previous patches 2 - prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t 3 - Should be green 4 - high fives! Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Tests pass, but I can't get authorities to sort. I am sorry, can you please check if it works for you? I tried ... resetting indexes, reindexing multiple times, multiple reset_all, really stuck here :( Giving this another go... keep fingers crossed! Nick and I found the key: DELETE from search_marc_to_field; DELETE from search_marc_map; delete from search_field; localhost:8081/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 perl misc/search_tools/rebuildelasticsearch.pl -v -d I can search authorities now, sorting them as I see fit :) Created attachment 77088 [details] [review] Bug 19604: Elasticsearch Fixes for build_authorities_query for auth searching To test: 1 - Do some authority searches in Zebra 2 - Switch to ES and repeat, results will vary and some may fail 3 - Apply patch and dependencies 4 - Reindex ES 5 - Repeat searches, they should suceed and results should be similar to Zebra 6 - Slight differences are okay, but results should (mostly) meet expectations A few notes: We add a 'normalizer' to ensure we get a single token from the heading indexes, this makes 'starts with' work as expcted We switch to 'AND' for fields searched from cataloging editor - this matches Zebra results We force the '__sort' fields for sorting - if sorting looks wrong try reducing the heading field to a single subfield - this will need to be addressed on a future bug (multiple subfields create an array, ES sorts those randomly) Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 77089 [details] [review] Bug 19604: Unit tests 1 - Apply previous patches 2 - prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t 3 - Should be green 4 - high fives! Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Awesome work all! Pushed to master for 18.11 Created attachment 77328 [details] [review] Bug 19604: Fix perlcritic "Loop iterator is not lexical" Loop iterator is not lexical at line 318, column 13. See page 108 of PBP. (Severity: 5) Follow-up pushed to master. Gracias, Jonathan! This looks a lot like a bug rather than an enhancement, however with it depending on bug 20073 which certainly is an enhancement I am struggling to apply it to the 18.05.x branch. Can anyone more familiar with ES confirm whether 18.05.x is affected? Ay up Martin, When I did test it against 17.12 it was very relevant and I quickly rebased it on 18.05.00 to have it in our production 18.05 before it will be rebased on Bug 20073. Nick should confirm this, but it seems to me his last patch is only a rebase against Bug 20073, so maybe you could use the previous patch, or what I have done : https://github.com/BULAC/koha/commits/18.05.00_bug_19604 But maybe I am suggesting something terrible in the whole development process? (In reply to Nicolas Legrand from comment #30) > Ay up Martin, > > When I did test it against 17.12 it was very relevant and I quickly rebased > it on 18.05.00 to have it in our production 18.05 before it will be rebased > on Bug 20073. > > Nick should confirm this, but it seems to me his last patch is only a rebase > against Bug 20073, so maybe you could use the previous patch, or what I have > done : https://github.com/BULAC/koha/commits/18.05.00_bug_19604 > > But maybe I am suggesting something terrible in the whole development > process? This is what I was going to suggest as well - it is mostly moving the changes from the module into the files, earlier patches should work, just check the tests to ensure nothing else changed and let me knwo if you need me to take a crack Excellent, thanks Nicolas and Nick. I thought it was probably as simple as that but as I'm not fully up to speed on ES myself yet it was good to have it verified. The repo link is very helpful, I'll get this pushed today :) Pushed to 18.05.x for 18.05.03. Thanks guys. *** Bug 21103 has been marked as a duplicate of this bug. *** Enhancement not pushed to 17.11.x |