From 31f5549522e20c502002a99bd2da3b03a3e8ced7 Mon Sep 17 00:00:00 2001 From: Kevin Carnes 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 --- .../Elasticsearch/QueryBuilder.pm | 3 + .../SearchEngine/Elasticsearch/QueryBuilder.t | 78 +++++++++++-------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm index dba475eb3c..f87a6b483b 100644 --- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm +++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm @@ -306,6 +306,9 @@ 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) ); + if ($search_param_query_str) { + $search_param_query_str = "($search_param_query_str)"; + } $query_str = join( ' AND ', $search_param_query_str || (), $self->_join_queries( $self->_convert_index_strings(@$limits) ) || () ); diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t index b4e2de938c..ee97c4a6d9 100755 --- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t +++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t @@ -217,7 +217,7 @@ subtest 'build_authorities_query_compat() tests' => sub { }; subtest 'build_query tests' => sub { - plan tests => 57; + plan tests => 59; my $qb; @@ -276,68 +276,68 @@ subtest 'build_query tests' => sub { ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'] ); is( $query->{query}{query_string}{query}, - "(donald duck)", + "((donald duck))", "query not altered if QueryAutoTruncate disabled" ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'], ['kw,phr'] ); is( $query->{query}{query_string}{query}, - '("donald duck")', + '(("donald duck"))', "keyword as phrase correctly quotes search term and strips index" ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'], ['title'] ); is( $query->{query}{query_string}{query}, - '(title:(donald duck))', + '((title:(donald duck)))', 'multiple words in a query term are enclosed in parenthesis' ); ( undef, $query ) = $qb->build_query_compat( ['AND'], ['donald duck', 'disney'], ['title', 'author'] ); is( $query->{query}{query_string}{query}, - '(title:(donald duck)) AND (author:disney)', + '((title:(donald duck)) AND (author:disney))', 'multiple query terms are enclosed in parenthesis while a single one is not' ); my ($simple_query, $query_cgi, $query_desc); ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['"donald duck"', 'walt disney'], ['ti', 'au'] ); is($query_cgi, 'idx=ti&q=%22donald%20duck%22&idx=au&q=walt%20disney', 'query cgi ok for multiterm query'); - is($query_desc, '(title:("donald duck")) (author:(walt disney))', 'query desc ok for multiterm query'); + is($query_desc, '((title:("donald duck")) (author:(walt disney)))', 'query desc ok for multiterm query'); ( undef, $query ) = $qb->build_query_compat( undef, ['2019'], ['yr,st-year'] ); is( $query->{query}{query_string}{query}, - '(date-of-publication:2019)', + '((date-of-publication:2019))', 'Year in an st-year search is handled properly' ); ( undef, $query ) = $qb->build_query_compat( undef, ['2018-2019'], ['yr,st-year'] ); is( $query->{query}{query_string}{query}, - '(date-of-publication:[2018 TO 2019])', + '((date-of-publication:[2018 TO 2019]))', 'Year range in an st-year search is handled properly' ); ( undef, $query ) = $qb->build_query_compat( undef, ['-2019'], ['yr,st-year'] ); is( $query->{query}{query_string}{query}, - '(date-of-publication:[* TO 2019])', + '((date-of-publication:[* TO 2019]))', 'Open start year in year range of an st-year search is handled properly' ); ( undef, $query ) = $qb->build_query_compat( undef, ['2019-'], ['yr,st-year'] ); is( $query->{query}{query_string}{query}, - '(date-of-publication:[2019 TO *])', + '((date-of-publication:[2019 TO *]))', 'Open end year in year range of an st-year search is handled properly' ); ( undef, $query ) = $qb->build_query_compat( undef, ['2019-'], ['yr,st-year'], ['yr,st-numeric=-2019'] ); is( $query->{query}{query_string}{query}, - '(date-of-publication:[2019 TO *]) AND date-of-publication:[* TO 2019]', + '((date-of-publication:[2019 TO *])) AND date-of-publication:[* TO 2019]', 'Open end year in year range of an st-year search is handled properly' ); @@ -347,7 +347,7 @@ subtest 'build_query tests' => sub { ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'] ); is( $query->{query}{query_string}{query}, - "(donald* duck*)", + "((donald* duck*))", "simple query is auto truncated when QueryAutoTruncate enabled" ); @@ -356,35 +356,35 @@ subtest 'build_query tests' => sub { ['donald or duck and mickey not mouse'] ); is( $query->{query}{query_string}{query}, - "(donald* or duck* and mickey* not mouse*)", + "((donald* or duck* and mickey* not mouse*))", "reserved words are not affected by QueryAutoTruncate" ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald* duck*'] ); is( $query->{query}{query_string}{query}, - "(donald* duck*)", + "((donald* duck*))", "query with '*' is unaltered when QueryAutoTruncate is enabled" ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck and the mouse'] ); is( $query->{query}{query_string}{query}, - "(donald* duck* and the* mouse*)", + "((donald* duck* and the* mouse*))", "individual words are all truncated and stopwords ignored" ); ( undef, $query ) = $qb->build_query_compat( undef, ['*'] ); is( $query->{query}{query_string}{query}, - "(*)", + "((*))", "query of just '*' is unaltered when QueryAutoTruncate is enabled" ); ( undef, $query ) = $qb->build_query_compat( undef, ['"donald duck"'], undef, ['available'] ); is( $query->{query}{query_string}{query}, - '("donald duck") AND onloan:false', + '(("donald duck")) AND onloan:false', "query with quotes is unaltered when QueryAutoTruncate is enabled" ); @@ -392,98 +392,98 @@ subtest 'build_query tests' => sub { ( undef, $query ) = $qb->build_query_compat( undef, ['"donald duck" and "the mouse"'] ); is( $query->{query}{query_string}{query}, - '("donald duck" and "the mouse")', + '(("donald duck" and "the mouse"))', "all quoted strings are unaltered if more than one in query" ); ( undef, $query ) = $qb->build_query_compat( undef, ['barcode:123456'] ); is( $query->{query}{query_string}{query}, - '(barcode:123456*)', + '((barcode:123456*))', "query of specific field is truncated" ); ( undef, $query ) = $qb->build_query_compat( undef, ['Local-number:"123456"'] ); is( $query->{query}{query_string}{query}, - '(local-number:"123456")', + '((local-number:"123456"))', "query of specific field including hyphen and quoted is not truncated, field name is converted to lower case" ); ( undef, $query ) = $qb->build_query_compat( undef, ['Local-number:123456'] ); is( $query->{query}{query_string}{query}, - '(local-number:123456*)', + '((local-number:123456*))', "query of specific field including hyphen and not quoted is truncated, field name is converted to lower case" ); ( undef, $query ) = $qb->build_query_compat( undef, ['Local-number.raw:123456'] ); is( $query->{query}{query_string}{query}, - '(local-number.raw:123456*)', + '((local-number.raw:123456*))', "query of specific field including period and not quoted is truncated, field name is converted to lower case" ); ( undef, $query ) = $qb->build_query_compat( undef, ['Local-number.raw:"123456"'] ); is( $query->{query}{query_string}{query}, - '(local-number.raw:"123456")', + '((local-number.raw:"123456"))', "query of specific field including period and quoted is not truncated, field name is converted to lower case" ); ( undef, $query ) = $qb->build_query_compat( undef, ['J.R.R'] ); is( $query->{query}{query_string}{query}, - '(J.R.R*)', + '((J.R.R*))', "query including period is truncated but not split at periods" ); ( undef, $query ) = $qb->build_query_compat( undef, ['title:"donald duck"'] ); is( $query->{query}{query_string}{query}, - '(title:"donald duck")', + '((title:"donald duck"))', "query of specific field is not truncated when surrounded by quotes" ); ( undef, $query ) = $qb->build_query_compat( undef, ['donald duck'], ['title'] ); is( $query->{query}{query_string}{query}, - '(title:(donald* duck*))', + '((title:(donald* duck*)))', 'words of a multi-word term are properly truncated' ); ( undef, $query ) = $qb->build_query_compat( ['AND'], ['donald duck', 'disney'], ['title', 'author'] ); is( $query->{query}{query_string}{query}, - '(title:(donald* duck*)) AND (author:disney*)', + '((title:(donald* duck*)) AND (author:disney*))', 'words of a multi-word term and single-word term are properly truncated' ); ( undef, $query ) = $qb->build_query_compat( undef, ['title:"donald duck"'], undef, undef, undef, undef, undef, { suppress => 1 } ); is( $query->{query}{query_string}{query}, - '(title:"donald duck") AND suppress:false', + '((title:"donald duck")) AND suppress:false', "query of specific field is added AND suppress:false" ); ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['title:"donald duck"'], undef, undef, undef, undef, undef, { suppress => 0 } ); is( $query->{query}{query_string}{query}, - '(title:"donald duck")', + '((title:"donald duck"))', "query of specific field is not added AND suppress:0" ); ( undef, $query ) = $qb->build_query_compat( ['AND'], ['title:"donald duck"'], undef, ['author:Dillinger Escaplan'] ); is( $query->{query}{query_string}{query}, - '(title:"donald duck") AND author:("Dillinger Escaplan")', + '((title:"donald duck")) AND author:("Dillinger Escaplan")', "Simple query with limit term quoted in parentheses" ); ( undef, $query ) = $qb->build_query_compat( ['AND'], ['title:"donald duck"'], undef, ['author:Dillinger Escaplan', 'itype:BOOK'] ); is( $query->{query}{query_string}{query}, - '(title:"donald duck") AND (author:("Dillinger Escaplan")) AND (itype:("BOOK"))', + '((title:"donald duck")) AND (author:("Dillinger Escaplan")) AND (itype:("BOOK"))', "Simple query with each limit's term quoted in parentheses" ); is($query_cgi, 'idx=&q=title%3A%22donald%20duck%22', 'query cgi'); @@ -492,10 +492,24 @@ subtest 'build_query tests' => sub { ( undef, $query ) = $qb->build_query_compat( ['AND'], ['title:"donald duck"'], undef, ['author:Dillinger Escaplan', 'mc-itype,phr:BOOK', 'mc-itype,phr:CD'] ); is( $query->{query}{query_string}{query}, - '(title:"donald duck") AND (author:("Dillinger Escaplan")) AND itype:(("BOOK") OR ("CD"))', + '((title:"donald duck")) AND (author:("Dillinger Escaplan")) AND itype:(("BOOK") OR ("CD"))', "Limits quoted correctly when passed as phrase" ); + ( undef, $query ) = $qb->build_query_compat( ['OR'], ['title:"donald duck"', 'author:"Dillinger Escaplan"'], undef, ['itype:BOOK'] ); + is( + $query->{query}{query_string}{query}, + '((title:"donald duck") OR (author:"Dillinger Escaplan")) AND itype:("BOOK")', + "OR query with limit" + ); + + ( undef, $query ) = $qb->build_query_compat( undef, undef, undef, ['itype:BOOK'] ); + is( + $query->{query}{query_string}{query}, + 'itype:("BOOK")', + "Limit only" + ); + # Scan queries ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['new'], ['au'], undef, undef, 1 ); is( -- 2.17.1