Attachment 135360 Details for Bug 30152 – Bug 30152: Elasticsearch - queries with OR don't work with limits

[patch] Bug 30152: Elasticsearch - queries with OR don't work with limits

Bug-30152-Elasticsearch---queries-with-OR-dont-wor.patch (text/plain), 13.32 KB, created by Kevin Carnes on 2022-05-26 08:38:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kevin Carnes

Created: 2022-05-26 08:38:03 UTC

Size: 13.32 KB

patch

obsolete

>From 2ce583333e25b2f6c5642c9822135ba31b4092c4 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
>---
> .../Elasticsearch/QueryBuilder.pm             |  1 +
> .../SearchEngine/Elasticsearch/QueryBuilder.t | 71 ++++++++++---------
> 2 files changed, 40 insertions(+), 32 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index dba475eb3c..5934956326 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -306,6 +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) );
>+        $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..33349c2a00 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 => 58;
> 
>     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,17 @@ 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"
>+    );
>+
>     # Scan queries
>     ( undef, $query, $simple_query, $query_cgi, $query_desc ) = $qb->build_query_compat( undef, ['new'], ['au'], undef, undef, 1 );
>     is(
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 30152: 131010 | 131011 | 131017 | 131018 | 135334 | 135360 | 135446 | 135454 | 135793 | 136569 | 136766