Bug 27070 - Elasticsearch - with Elasticsearch 6 searches failing unless all terms are in the same field
Summary: Elasticsearch - with Elasticsearch 6 searches failing unless all terms are in...
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-20 19:09 UTC by Nick Clemens
Modified: 2020-11-26 09:07 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 27070: Add cross_fields type to our searches (1.28 KB, patch)
2020-11-20 19:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27070: Add cross_fields type to our searches (1.82 KB, patch)
2020-11-20 19:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27070: Add cross_fields type to our searches (1.34 KB, patch)
2020-11-21 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27070: Add cross_fields type to our searches (1.39 KB, patch)
2020-11-21 13:17 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27070: Add cross_fields type to our searches (1.45 KB, patch)
2020-11-21 18:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 27070: Add cross_fields type to our searches (1.60 KB, patch)
2020-11-25 12:45 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-11-20 19:09:50 UTC
To recreate - be using Elasticsearch 6 in KOha

On sample data try searching for:
street shuffle

cool, that works

Now try searching for a word from title and the publisher name:
shuffle constable

No results, odd.

Try searching for:
shuffle AND constable

That works.

I cannot figure from the docs why this is the case - we pass a default operator, that should be enough I thought, but no.

https://www.elastic.co/guide/en/elasticsearch/reference/6.8/query-dsl-query-string-query.html#query-string-syntax

for comparison:
https://www.elastic.co/guide/en/elasticsearch/reference/5.6/query-dsl-query-string-query.html


After testing the type => 'cross_fields' seems to do what we want
Comment 1 Nick Clemens 2020-11-20 19:11:32 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2020-11-20 19:14:20 UTC Comment hidden (obsolete)
Comment 3 Didier Gautheron 2020-11-21 12:36:00 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2020-11-21 13:17:03 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens 2020-11-21 13:17:59 UTC
Created attachment 113908 [details] [review]
Bug 27070: Add cross_fields type to our searches

This patch adds the 'cross_fields' type to our searches:
https://www.elastic.co/guide/en/elasticsearch/reference/6.8/query-dsl-query-string-query.html#query-string-syntax

Without this patch the search terms seem to all require being in the same field when using Elasticsearch 6

To test:
0 - Set QueryAutoTruncate to 'only if * is added'
1 - Find a record with a title and publisher
2 - Search for a word form the title and confirm the record is returned
3 - Search for a work from the title and the publisher's name
4 - The record is not returned
5 - Apply patch
6 - Repeat #3
7 - The record is returned
Comment 6 Victor Grousset/tuxayo 2020-11-21 18:59:43 UTC
Created attachment 113911 [details] [review]
Bug 27070: Add cross_fields type to our searches

This patch adds the 'cross_fields' type to our searches:
https://www.elastic.co/guide/en/elasticsearch/reference/6.8/query-dsl-query-string-query.html#query-string-syntax

Without this patch the search terms seem to all require being in the same field when using Elasticsearch 6

To test:
0 - Set QueryAutoTruncate to 'only if * is added'
1 - Find a record with a title and publisher
2 - Search for a word form the title and confirm the record is returned
3 - Search for a work from the title and the publisher's name
4 - The record is not returned
5 - Apply patch
6 - Repeat #3
7 - The record is returned

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Joonas Kylmälä 2020-11-24 12:58:41 UTC
Any idea how well this works with weighted search fields? There was some discussion in https://discuss.elastic.co/t/cross-fields-and-boost-is-it-feasible/134219/9 about it and also https://www.elastic.co/guide/en/elasticsearch/reference/master/query-dsl-multi-match-query.html#type-cross-fields mentions it but I don't see any immediate issues in our use case.
Comment 8 Fridolin Somers 2020-11-24 13:18:57 UTC
Whooo I was looking for this fix since weeks, thanks a lot
Comment 9 Nick Clemens 2020-11-24 13:51:04 UTC
(In reply to Joonas Kylmälä from comment #7)
> Any idea how well this works with weighted search fields? There was some
> discussion in
> https://discuss.elastic.co/t/cross-fields-and-boost-is-it-feasible/134219/9
> about it and also
> https://www.elastic.co/guide/en/elasticsearch/reference/master/query-dsl-
> multi-match-query.html#type-cross-fields mentions it but I don't see any
> immediate issues in our use case.

It seems mostly that they are saying cross_fields does more than just searching all fields, it tries to guess which fields are most relevant to each term and boost them - so adding field boosts on top of that kind of ruins the work.

The problem I see is that the multi field query says it should look for each term in each field with 'OR', i.e.
(content:this OR name:this) AND (content:that OR name:that)
but what is returned actually seems to be:
(content:this AND that) AND (name:this AND that)

possibly the problem is with our 'default_operator' setting of 'AND'

I think with OR we might end up with results that don't contain all terms, but the ones that do should be most relevant, but our field boosting may push partial matches to the top.

cross_fields seems to at least generate the search we expect right now. I am open to discussion, but the current state here is problematic for many users
Comment 10 Joonas Kylmälä 2020-11-25 12:45:04 UTC
Created attachment 113976 [details] [review]
Bug 27070: Add cross_fields type to our searches

This patch adds the 'cross_fields' type to our searches:
https://www.elastic.co/guide/en/elasticsearch/reference/6.8/query-dsl-query-string-query.html#query-string-syntax

Without this patch the search terms seem to all require being in the same field when using Elasticsearch 6

To test:
0 - Set QueryAutoTruncate to 'only if * is added'
1 - Find a record with a title and publisher
2 - Search for a word form the title and confirm the record is returned
3 - Search for a work from the title and the publisher's name
4 - The record is not returned
5 - Apply patch
6 - Repeat #3
7 - The record is returned

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 11 Joonas Kylmälä 2020-11-25 12:49:30 UTC
Using cross_fields here makes this now so that it doesn't try to do an AND search on one field only for all the search keywords which is pretty much guaranteed to fail as people here noticed. The searches now at least bring all the results that contain those keywords, when previously very much relevant results were missing! I'm passing QA for this given it is a big improvement to the previous state but in the future we still need to verify and read the Elasticsearch docs if there could be a even better way doing searches.
Comment 12 Jonathan Druart 2020-11-25 13:07:10 UTC
Joonas, could you open a follow-up bug report with the links/ideas to explore?
Comment 13 Joonas Kylmälä 2020-11-25 13:30:35 UTC
(In reply to Jonathan Druart from comment #12)
> Joonas, could you open a follow-up bug report with the links/ideas to
> explore?

I opened bug 27091. It's gonna be tedious process but I'm actually planning to start doing in the following 3 months, hopefully also being ready then :)
Comment 14 Jonathan Druart 2020-11-25 15:20:07 UTC
Pushed to master for 20.11, thanks to everybody involved!