Attachment 136766 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), 3.82 KB, created by Fridolin Somers on 2022-06-30 05:46:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Fridolin Somers

Created: 2022-06-30 05:46:16 UTC

Size: 3.82 KB

patch

obsolete

>From f9e2985bc78364547448a346d4b44600a2b8853b 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
>
>Signed-off-by: David Nind <david@davidnind.com>
>Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
>---
> Koha/SearchEngine/Elasticsearch/QueryBuilder.pm  | 10 ++++++++--
> .../SearchEngine/Elasticsearch/QueryBuilder.t    | 16 +++++++++++++++-
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
>diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>index 7250ea6048..52a53d5dcf 100644
>--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
>@@ -305,10 +305,16 @@ sub build_query_compat {
>         # would be to pass them separately into build_query and let it build
>         # 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) );
>+        my @search_param_query_array = $self->_create_query_string(@search_params);
>+        $search_param_query_str = join( ' ', @search_param_query_array );
>+        my $search_param_limit_str =
>+          $self->_join_queries( $self->_convert_index_strings(@$limits) );
>+        if ( @search_param_query_array > 1 && $search_param_limit_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) ) || () );
>+            $search_param_limit_str || () );
> 
>         # If there's no query on the left, let's remove the junk left behind
>         $query_str =~ s/^ AND //;
>diff --git a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
>index 4e8964e682..c0084653c2 100755
>--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
>+++ b/t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
>@@ -215,7 +215,7 @@ subtest 'build_authorities_query_compat() tests' => sub {
> };
> 
> subtest 'build_query tests' => sub {
>-    plan tests => 57;
>+    plan tests => 59;
> 
>     my $qb;
> 
>@@ -494,6 +494,20 @@ subtest 'build_query tests' => sub {
>         "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.35.3

Actions: View | Diff | Splinter Review

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