|
Description
Kevin Carnes
2022-02-22 13:27:12 UTC
Created attachment 131010 [details] [review] When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Title) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeats the search 13) Observe that results with both keywords are in the results 14) Sign off Created attachment 131011 [details] [review] When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Title) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Created attachment 131017 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Comment on attachment 131017 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits >From b4cfe66eea7d100d319a5ee256061e8eea15bd99 Mon Sep 17 00:00:00 2001 >From: Kevin Carnes <kevin.carnes@ub.lu.se> >Date: Tue, 22 Feb 2022 14:29:15 +0100 >Subject: [PATCH] Bug 30152: Elasticsearch - queries with OR don't work with > limits > >When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. > >To test: >1) Set SearchEngine to Elasticsearch >2) Index records in Elasticsearch >3) Do an advanced search >4) Select More options >5) Enter a value for the first Keyword (e.g. Novels) >6) Change "and" before the second Keyword to "or" >7) Enter another value for the second Keyword (e.g. Prose) >8) Limit the search (e.g. Item type Books) >9) Do the search >10) Observe that records with the first keyword are not in the results >11) Apply the patch >12) Repeat the search >13) Observe that results with both keywords are in the results >14) Sign off > >Sponsored-by: Lund University Library >--- > Koha/SearchEngine/Elasticsearch/QueryBuilder.pm | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >index 3e2cb1b784..b2ef8aa8d9 100644 >--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >@@ -306,9 +306,7 @@ sub build_query_compat { > # them into a structured ES query itself. Maybe later, though that'd be > # more robust. > $search_param_query_str = join( ' ', $self->_create_query_string(@search_params) ); >- $query_str = join( ' AND ', >- $search_param_query_str || (), >- $self->_join_queries( $self->_convert_index_strings(@$limits) ) || () ); >+ $query_str = join( ' AND ', map { "($_)" } grep { $_ } ( $search_param_query_str, $self->_join_queries( $self->_convert_index_strings(@$limits) ) ) ); > > # If there's no query on the left, let's remove the junk left behind > $query_str =~ s/^ AND //; >-- >2.17.1 Created attachment 131018 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library *** Bug 30173 has been marked as a duplicate of this bug. *** Good catch. This will need a unit test. In my opinion this code looks better (if it fixes the same way) : https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30141#c1 Created attachment 135334 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library (In reply to Fridolin Somers from comment #8) > In my opinion this code looks better (if it fixes the same way) : > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30141#c1 Ok, it now uses that code and has a simple test to make sure that the query looks correct, but doesn't actually test searching. Created attachment 135360 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Created attachment 135446 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Created attachment 135454 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Signed-off-by: David Nind <david@davidnind.com> Looks good but I propose we had those parenthesis only when there are limits. Created attachment 135793 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Looks great Looks great Kevin :D Created attachment 136569 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Signed-off-by: David Nind <david@davidnind.com> Created attachment 136766 [details] [review] Bug 30152: Elasticsearch - queries with OR don't work with limits When a query with "OR" is combined with a limit in Elasticsearch, the precedence is not preserved and the results are not correct. To test: 1) Set SearchEngine to Elasticsearch 2) Index records in Elasticsearch 3) Do an advanced search 4) Select More options 5) Enter a value for the first Keyword (e.g. Novels) 6) Change "and" before the second Keyword to "or" 7) Enter another value for the second Keyword (e.g. Prose) 8) Limit the search (e.g. Item type Books) 9) Do the search 10) Observe that records with the first keyword are not in the results 11) Apply the patch 12) Repeat the search 13) Observe that results with both keywords are in the results 14) Sign off Sponsored-by: Lund University Library Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Pushed to master for 22.11. Nice work everyone, thanks! Backported to 22.05.x for 22.05.04 pushed to 21.11.x for 21.11.11 Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved. |