Bugzilla – Attachment 101002 Details for
Bug 24902
Elasticsearch - different limits are joined with OR instead of AND
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 24902: Join different mc- limits with AND (elasticsearch)
Bug-24902-Join-different-mc--limits-with-AND-elast.patch (text/plain), 5.28 KB, created by
Julian Maurice
on 2020-03-19 09:58:18 UTC
(
hide
)
Description:
Bug 24902: Join different mc- limits with AND (elasticsearch)
Filename:
MIME Type:
Creator:
Julian Maurice
Created:
2020-03-19 09:58:18 UTC
Size:
5.28 KB
patch
obsolete
>From ea988f1bd1115ed2df27b0154e6b605b91ebce27 Mon Sep 17 00:00:00 2001 >From: Julian Maurice <julian.maurice@biblibre.com> >Date: Thu, 19 Mar 2020 09:38:52 +0100 >Subject: [PATCH] Bug 24902: Join different mc- limits with AND (elasticsearch) >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >In the advanced search form, you can enable several limits using syspref >AdvancedSearchTypes (namely itemtypes, shelving locations, collections) >When used, the resulting query parts end up being joined with OR, even >if the field is different. That means that if you pick "Book" under >itemtypes tab, and "Fiction" under collection tab, it will search >"itype:BOOK OR ccode:FIC". It should be AND. > >For instance, if you select: > Itemtypes: > â Book > â DVD > Location: > â Child > â Adult > >it should search: > itype:(Book OR DVD) AND location:(Child OR Adult) > >Test plan: >0. Do not apply the patch yet >1. Enable elasticsearch >2. Set syspref AdvancedSearchTypes = 'itemtypes|loc|ccode' >3. Create a new itemtype and a new authorised value for categories LOC > and CCODE >4. Create a biblio with the new itemtype, another biblio with the new > location, another biblio with the new collection, and again another > biblio with the new itemtype, location and collection >5. Verify that you can find these new biblio records using only the > "advanced search types" in the advanced search form >6. In the advanced search form, pick all 3 limits (itemtype, location, > collection) and verify that it returns the 4 records. >7. Apply the patch >8. Repeat step 6, it should now return only the biblio that satisfies > all criteria >9. Verify that if you select more than one > {itemtype|location|collection} it still returns results that > satisfies any selected criteria >10. prove t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t >--- > .../Elasticsearch/QueryBuilder.pm | 26 ++++++++++------ > .../SearchEngine/ElasticSearch/QueryBuilder.t | 31 ++++++++++++++++++- > 2 files changed, 46 insertions(+), 11 deletions(-) > >diff --git a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >index 166121fa11..1bbbf079b8 100644 >--- a/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >+++ b/Koha/SearchEngine/Elasticsearch/QueryBuilder.pm >@@ -853,16 +853,22 @@ sub _join_queries { > map { s/^mc-//r } grep { defined($_) && $_ ne '' && $_ =~ /^mc-/ } @parts; > return () unless @norm_parts + @mc_parts; > return ( @norm_parts, @mc_parts )[0] if @norm_parts + @mc_parts == 1; >- my $grouped_mc = >- @mc_parts ? '(' . ( join ' OR ', map { "($_)" } @mc_parts ) . ')' : (); >- >- # Handy trick: $x || () inside a join means that if $x ends up as an >- # empty string, it gets replaced with (), which makes join ignore it. >- # (bad effect: this'll also happen to '0', this hopefully doesn't matter >- # in this case.) >- join( ' AND ', >- join( ' AND ', map { "($_)" } @norm_parts ) || (), >- $grouped_mc || () ); >+ >+ # Group limits by field, so they can be OR'ed together >+ my %mc_limits; >+ foreach my $mc_part (@mc_parts) { >+ my ($field, $value) = split /:/, $mc_part, 2; >+ $mc_limits{$field} //= []; >+ push @{ $mc_limits{$field} }, $value; >+ } >+ >+ @mc_parts = map { >+ sprintf('%s:(%s)', $_, join (' OR ', @{ $mc_limits{$_} })); >+ } sort keys %mc_limits; >+ >+ @norm_parts = map { "($_)" } @norm_parts; >+ >+ return join( ' AND ', @norm_parts, @mc_parts); > } > > =head2 _make_phrases >diff --git a/t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t b/t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t >index 3bd5e4244a..2057138431 100644 >--- a/t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t >+++ b/t/Koha/SearchEngine/ElasticSearch/QueryBuilder.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 5; >+use Test::More tests => 6; > use t::lib::Mocks; > > use_ok('Koha::SearchEngine::Elasticsearch::QueryBuilder'); >@@ -223,4 +223,33 @@ subtest '_clean_search_term() tests' => sub { > is($res, 'test another part', 'unbalanced curly brackets replaced correctly'); > }; > >+subtest '_join_queries' => sub { >+ plan tests => 6; >+ >+ my $params = { >+ index => $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX, >+ }; >+ my $qb = Koha::SearchEngine::Elasticsearch::QueryBuilder->new($params); >+ >+ my $query; >+ >+ $query = $qb->_join_queries('foo'); >+ is($query, 'foo', 'should work with a single param'); >+ >+ $query = $qb->_join_queries(undef, '', 'foo', '', undef); >+ is($query, 'foo', 'should ignore undef or empty queries'); >+ >+ $query = $qb->_join_queries('foo', 'bar'); >+ is($query, '(foo) AND (bar)', 'should join queries with an AND'); >+ >+ $query = $qb->_join_queries('homebranch:foo', 'onloan:false'); >+ is($query, '(homebranch:foo) AND (onloan:false)', 'should also work when field is specified'); >+ >+ $query = $qb->_join_queries('homebranch:foo', 'mc-itype:BOOK', 'mc-itype:EBOOK'); >+ is($query, '(homebranch:foo) AND itype:(BOOK OR EBOOK)', 'should join with OR when using an "mc-" field'); >+ >+ $query = $qb->_join_queries('homebranch:foo', 'mc-itype:BOOK', 'mc-itype:EBOOK', 'mc-location:SHELF'); >+ is($query, '(homebranch:foo) AND itype:(BOOK OR EBOOK) AND location:(SHELF)', 'should join "mc-" parts with AND if not the same field'); >+}; >+ > 1; >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 24902
:
101002
|
101353
|
101363