Summary: | Elasticsearch - with Elasticsearch 6 searches failing unless all terms are in the same field | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | Searching - Elasticsearch | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | major | ||
Priority: | P5 - low | CC: | alex.arnaud, andrew, didier.gautheron, fridolin.somers, jonathan.druart, joonas.kylmala, julian.maurice, martin.renvoize, mkstephens, severine.queune, tomascohen, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27091 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
20.11.00, 20.05.07, 19.11.14
|
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 27252 | ||
Attachments: |
Bug 27070: Add cross_fields type to our searches
Bug 27070: Add cross_fields type to our searches Bug 27070: Add cross_fields type to our searches Bug 27070: Add cross_fields type to our searches Bug 27070: Add cross_fields type to our searches Bug 27070: Add cross_fields type to our searches |
Description
Nick Clemens (kidclamp)
2020-11-20 19:09:50 UTC
Created attachment 113902 [details] [review] Bug 27070: Add cross_fields type to our searches Created attachment 113903 [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: 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 Hi It seems diff --git a/C4/Circulation.pm b/C4/Circulation.pm is an unrelated leftover, it adds an unused reasons hash. Created attachment 113907 [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: 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 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 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> 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. Whooo I was looking for this fix since weeks, thanks a lot (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 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> 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. Joonas, could you open a follow-up bug report with the links/ideas to explore? (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 :) Pushed to master for 20.11, thanks to everybody involved! Pushed to 20.05.x for 20.05.07 There is an issue here, this breaks searching with Elasticsearch 5.x It may be okay, ES5 is no longer supported, so this may be the point at which we want to support only Es6+, but we will need to announce that EOL since 2019-03-11 IIUC https://www.elastic.co/support/eol (In reply to Nick Clemens from comment #16) > There is an issue here, this breaks searching with Elasticsearch 5.x > > It may be okay, ES5 is no longer supported, so this may be the point at > which we want to support only Es6+, but we will need to announce that In that case I am suggesting to remove it from 20.05, before .7 is released. And add a warning on the about page, see bug 27252. Does this mean that for now, 19.11 and 20.05 will have to advertise (and test in the CI) ES 5 only? > Does this mean that for now, 19.11 and 20.05 will have to advertise (and test in the CI) ES 5 only? Nope, they can support ES 6 and ES 5 thanks to bug 27252 Backported: Pushed to 19.11.x branch for 19.11.14 |