Bug 19559 - Elasticsearch QueryAutoTruncate truncate field names with hyphens if data is quoted
Summary: Elasticsearch QueryAutoTruncate truncate field names with hyphens if data is ...
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: Julian Maurice
URL:
Whiteboard:
Keywords:
: 19565 (view as bug list)
Depends on: 18374
Blocks: 18131 19580
  Show dependency treegraph
 
Reported: 2017-11-01 15:46 UTC by Nick Clemens
Modified: 2017-11-26 16:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19559 - Unit tests (1.63 KB, patch)
2017-11-01 15:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19559 - Add '-' to list of characters we don't split search terms on (1.38 KB, patch)
2017-11-01 15:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19559 - Unit tests (1.69 KB, patch)
2017-11-03 17:57 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19559 - Add '-' to list of characters we don't split search terms on (1.44 KB, patch)
2017-11-03 17:57 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19559 - Unit tests (1.76 KB, patch)
2017-11-10 09:42 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 19559 - Add '-' to list of characters we don't split search terms on (1.51 KB, patch)
2017-11-10 09:42 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-11-01 15:46:47 UTC
With QueryAutoTruncate enabled a search for:
Local-number:"4"
is truncated as:
Local-* number:"4"
Comment 1 Nick Clemens 2017-11-01 15:51:06 UTC
Created attachment 68881 [details] [review]
Bug 19559 - Unit tests
Comment 2 Nick Clemens 2017-11-01 15:51:08 UTC
Created attachment 68882 [details] [review]
Bug 19559 - Add '-' to list of characters we don't split search terms on

Test plan assumes
QueryAutoTruncate = automatically
SearchEngine = Elasticsearch

To test:
0 - Apply Unit test patch only
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - Should fail
3 - Apply this patch
4 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
5 - should pass
6 - search for 'Local-number:"4"' (or a vlid biblionumber)
7 - should get expected result
Comment 3 David Gustafsson 2017-11-02 15:01:25 UTC
*** Bug 19565 has been marked as a duplicate of this bug. ***
Comment 4 David Gustafsson 2017-11-02 15:03:26 UTC
Would be nice if "." could as be included as a valid field characters, since it is used for subfields in Elasticsearch. ("Control-number.raw" for example).
Comment 5 David Bourgault 2017-11-03 17:57:47 UTC
Created attachment 68924 [details] [review]
Bug 19559 - Unit tests

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 6 David Bourgault 2017-11-03 17:57:49 UTC
Created attachment 68925 [details] [review]
Bug 19559 - Add '-' to list of characters we don't split search terms on

Test plan assumes
QueryAutoTruncate = automatically
SearchEngine = Elasticsearch

To test:
0 - Apply Unit test patch only
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - Should fail
3 - Apply this patch
4 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
5 - should pass
6 - search for 'Local-number:"4"' (or a vlid biblionumber)
7 - should get expected result

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 7 David Bourgault 2017-11-03 17:58:21 UTC
Works as described in test plan.
Comment 8 Nick Clemens 2017-11-04 01:33:35 UTC
(In reply to David Gustafsson from comment #4)
> Would be nice if "." could as be included as a valid field characters, since
> it is used for subfields in Elasticsearch. ("Control-number.raw" for
> example).

This could be done, opening a new bug 19580

We may need more adjustments to when we do and do not truncate/split
Comment 9 Julian Maurice 2017-11-10 09:42:50 UTC
Created attachment 69075 [details] [review]
Bug 19559 - Unit tests

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 10 Julian Maurice 2017-11-10 09:42:54 UTC
Created attachment 69076 [details] [review]
Bug 19559 - Add '-' to list of characters we don't split search terms on

Test plan assumes
QueryAutoTruncate = automatically
SearchEngine = Elasticsearch

To test:
0 - Apply Unit test patch only
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - Should fail
3 - Apply this patch
4 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
5 - should pass
6 - search for 'Local-number:"4"' (or a vlid biblionumber)
7 - should get expected result

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 11 Jonathan Druart 2017-11-26 16:26:33 UTC
Pushed to master for 17.11, thanks to everybody involved!