Bug 22592 - Elasticsearch - Support for index scan
Summary: Elasticsearch - Support for index scan
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P3 enhancement (vote)
Assignee: Ere Maijala
QA Contact: Alex Arnaud
URL:
Keywords:
Depends on: 22413
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-27 12:27 UTC by Ere Maijala
Modified: 2019-11-03 07:52 UTC (History)
8 users (show)

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


Attachments
Bug 22592: Add index scan emulation to Elasticsearch (13.87 KB, patch)
2019-03-28 11:51 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.14 KB, patch)
2019-03-28 11:51 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (13.87 KB, patch)
2019-04-03 11:21 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.14 KB, patch)
2019-04-03 11:21 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (13.94 KB, patch)
2019-04-09 18:49 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.19 KB, patch)
2019-04-09 18:49 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (13.94 KB, patch)
2019-04-25 13:27 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.42 KB, patch)
2019-04-25 13:27 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (14.01 KB, patch)
2019-04-25 18:52 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.47 KB, patch)
2019-04-25 18:52 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (13.86 KB, patch)
2019-08-21 08:15 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Fixes to index scan (3.47 KB, patch)
2019-08-21 08:15 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Rebase, add UNIMARC support and fix tests. (7.76 KB, patch)
2019-08-21 08:15 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (20.04 KB, patch)
2019-10-04 12:00 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (20.24 KB, patch)
2019-10-04 13:04 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 22592: Add index scan emulation to Elasticsearch (20.33 KB, patch)
2019-11-02 11:41 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ere Maijala 2019-03-27 12:27:27 UTC
We can emulate index scan at least to some extent by using faceting with an include filter.
Comment 1 Ere Maijala 2019-03-28 11:51:02 UTC
Created attachment 87135 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results
Comment 2 Ere Maijala 2019-03-28 11:51:05 UTC
Created attachment 87136 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.
Comment 3 Stefan Berndtsson 2019-04-03 11:12:47 UTC
I can't apply this patch. Do I need anything other than 22413 to make this work?

It fails with:

Applying: Bug 22592: Add index scan emulation to Elasticsearch
error: sha1 information is lacking or useless (Koha/SearchEngine/Elasticsearch/QueryBuilder.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 22592: Add index scan emulation to Elasticsearch
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-22592-Add-index-scan-emulation-to-Elasticsearc-iqm9C3.patch
Comment 4 Ere Maijala 2019-04-03 11:21:09 UTC
Created attachment 87344 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results
Comment 5 Ere Maijala 2019-04-03 11:21:12 UTC
Created attachment 87345 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.
Comment 6 Ere Maijala 2019-04-03 11:23:02 UTC
Stefan, thanks for testing. Nothing else is needed. I had to rebase, though, since there were changes in master. Just let bz follow the dependency and you should be good (for now...).
Comment 7 Michal Denar 2019-04-09 18:39:11 UTC
Hi Ere,
I'm not sure what but something make Elastic broken. I can!t reidex ES, U get:
Can't locate object method "throw" via package "Koha::Exceptions::Elasticsearch::MARCFieldExprParseError" (perhaps you forgot to load "Koha::Exceptions::Elasticsearch::MARCFieldExprParseError"?) at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 794.

I followed all dependencies.
Comment 8 Michal Denar 2019-04-09 18:47:20 UTC
Hi Ere,
I'm not sure what but something make Elastic broken. I can!t reidex ES, U get:
Can't locate object method "throw" via package "Koha::Exceptions::Elasticsearch::MARCFieldExprParseError" (perhaps you forgot to load "Koha::Exceptions::Elasticsearch::MARCFieldExprParseError"?) at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 794.

I followed all dependencies.
Comment 9 Michal Denar 2019-04-09 18:48:14 UTC
Sorry, T was some problem with my kohadevbox. I set Need signet off back.
Comment 10 Michal Denar 2019-04-09 18:49:46 UTC
Created attachment 87668 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 11 Michal Denar 2019-04-09 18:49:49 UTC
Created attachment 87669 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 12 Alex Arnaud 2019-04-25 09:55:25 UTC
Patch does not apply 

Applying: Bug 22592: Add index scan emulation to Elasticsearch
fatal: sha1 information is lacking or useless (Koha/SearchEngine/Elasticsearch/QueryBuilder.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 22592: Add index scan emulation to Elasticsearch
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-22592-Add-index-scan-emulation-to-Elasticsearc-ykcnua.patch
Comment 13 Ere Maijala 2019-04-25 13:27:42 UTC
Created attachment 88777 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 14 Ere Maijala 2019-04-25 13:27:46 UTC
Created attachment 88778 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 15 Michal Denar 2019-04-25 18:52:26 UTC
Created attachment 88807 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 16 Michal Denar 2019-04-25 18:52:29 UTC
Created attachment 88808 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 17 Alex Arnaud 2019-04-26 10:07:27 UTC
1) This feature is MARC21 specific. Doesn't work with UNIMARC.

2) Typo in the POD for _build_scan_query and _create_regex_filter (missing underscores)

3) Missing tests for _aggregation_scan / search_compat

4) 4 tests added in QueryBuilder.t but subtest's plan test not updated. Tests fail.
Comment 18 Ere Maijala 2019-08-21 08:15:04 UTC
Created attachment 92391 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 19 Ere Maijala 2019-08-21 08:15:08 UTC
Created attachment 92392 [details] [review]
Bug 22592: Fixes to index scan

Makes sure the scan term is displayed and redirection to record page is not used with index scan for a single result. Also adds a submit button so that you can submit a change for selected index or term easier.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 20 Ere Maijala 2019-08-21 08:15:15 UTC
Created attachment 92393 [details] [review]
Bug 22592: Rebase, add UNIMARC support and fix tests.
Comment 21 Brendan Gallagher 2019-10-04 07:38:48 UTC
I got to it too late - can you please rebase?
Comment 22 Ere Maijala 2019-10-04 12:00:52 UTC
Created attachment 93743 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 23 Ere Maijala 2019-10-04 12:04:43 UTC
Rebased and squashed the commits.

In addition there is one change to search.pl to make it only consider the first idx parameter for the scan index to use. For some reason this check was commented out since the introduction of the code block in commit https://github.com/Koha-Community/Koha/commit/9f3e6b4a407ff79a5fa9ff627077dee884bccc24, but without it the searches initiated from a scan field will use wrong search index. That's 11 years ago, which makes me wonder if something else broke it more recently, since otherwise it just means nobody is using the feature. ;)
Comment 24 Séverine Queune 2019-10-04 12:29:15 UTC
Hi Ere,
I've just tested with a sandbox and have an issue with the "Scan index" count, which is different of the total of results I get if I perform the search.
I wondered about comment 17 made by Alex : is it supposed to work with Unimarc ? Is it the origin of my issue ?
Thanks !
Comment 25 Ere Maijala 2019-10-04 12:36:58 UTC
The search it performs is currently not quite perfect match for the term list since search ignores punctuation etc. and may include results that start with the given term. Since the created search query does not carry information on the actual origin of the search, we can't handle it in a different way from a normal search in the ES code. I'm sure this could be further refined, but this is a minimal implementation to make not just fail completely.
Comment 26 Séverine Queune 2019-10-04 12:53:59 UTC
So if my understanding is good, you mean I can sign the patch, am I right ? :)
Comment 27 Ere Maijala 2019-10-04 12:54:53 UTC
Yes, exactly! :)
Comment 28 sandboxes@biblibre.com 2019-10-04 13:04:11 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 29 sandboxes@biblibre.com 2019-10-04 13:04:35 UTC
Created attachment 93745 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 30 Séverine Queune 2019-10-04 13:10:12 UTC
(In reply to Ere Maijala from comment #27)
> Yes, exactly! :)

Thank you Ere !
Comment 31 Katrin Fischer 2019-11-02 11:41:56 UTC
Created attachment 94962 [details] [review]
Bug 22592: Add index scan emulation to Elasticsearch

Adds support for using the "scan indexes" action in advanced search by using faceting with a prefix filter. Requires that the field be set as facetable for anything to be found.

Test plan:
1. Apply patch
2. Go to advanced search and click "More options"
3. Select author as the search field, enter a last name and check "Scan indexes"
4. Perform search and observe the result list resembling scan results

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2019-11-02 11:43:34 UTC
I feel while this is not the 'perfect' scan search, it's a great first step forwards from not supporting it to giving pretty good results in most cases (better than with Zebra actually).

I think we could even discuss for this to be a bug fix, as we offer the scan search with Elastic, but it didn't work before.
Comment 33 Martin Renvoize 2019-11-03 07:52:57 UTC
Nice work!

Pushed to master for 19.11.00