Bug 22524 - Elasticsearch - Date range in advanced search
Summary: Elasticsearch - Date range in advanced search
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: 18.05
Hardware: All All
: P5 - low minor (vote)
Assignee: Ere Maijala
QA Contact:
URL:
Keywords:
Depends on: 19575
Blocks: 22801
  Show dependency treegraph
 
Reported: 2019-03-15 18:06 UTC by Marjorie Barry-Vila
Modified: 2019-09-05 09:17 UTC (History)
9 users (show)

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


Attachments
Bug 22524: Fix publication date search with Elasticsearch (4.71 KB, patch)
2019-04-16 13:29 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22524: Fix publication date search with Elasticsearch (4.77 KB, patch)
2019-04-25 12:00 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22524: Fix publication date and year search with Elasticsearch (5.49 KB, patch)
2019-04-29 07:33 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22524: Fix publication date and year search with Elasticsearch (5.49 KB, patch)
2019-04-29 08:05 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22524: Fix date/time-last-modified search with Elasticsearch (1.90 KB, patch)
2019-04-29 08:05 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22524: Fix publication date and year search with Elasticsearch (5.56 KB, patch)
2019-04-30 19:33 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22524: Fix date/time-last-modified search with Elasticsearch (1.94 KB, patch)
2019-04-30 19:33 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22524: Fix publication date and year search with Elasticsearch (5.61 KB, patch)
2019-07-16 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22524: Fix date/time-last-modified search with Elasticsearch (2.00 KB, patch)
2019-07-16 18:44 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marjorie Barry-Vila 2019-03-15 18:06:32 UTC
Hi,
There is a problem with date range search in advanced search with ES.

Search "2001-2010" will give a list of results with 2001-2010 entered in marc field like 260$c2001-2010 or 264$2001-2010

Records with just 2001 in marc field will not be displayed in search results.


Maybe it's just a bad configuration in searchengine/elasticsearch/mappings.pl?

Regards,
Marjorie
Comment 1 Katrin Fischer 2019-03-27 06:41:01 UTC
That sounds like it's not using the controlled date but 260/264.
Comment 2 Frédéric Demians 2019-04-09 14:01:32 UTC
I have the same issue. Does it work somewhere? This seems to be a major regression.
Comment 3 Ere Maijala 2019-04-16 11:44:53 UTC
It's not really a mapping problem (though there may be additional mapping issues), but a problem in the Elasticsearch query parser not handling st-year query type.
Comment 4 Ere Maijala 2019-04-16 13:29:44 UTC
Created attachment 88063 [details] [review]
Bug 22524: Fix publication date search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-

Note that bug 22413 is needed for paging etc. to work properly.
Comment 5 Ere Maijala 2019-04-25 12:00:18 UTC
Created attachment 88758 [details] [review]
Bug 22524: Fix publication date search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-
Comment 6 Michal Denar 2019-04-26 19:40:56 UTC
Hi Ere,
I'm not sure why, but search with range don't work for me. Test passed. If I try 1989-2001 I get "No results match your search  with limit(s): 'copydate:1989-2001'. "

I chcecked all twoce but same results.


Mike
Comment 7 Ere Maijala 2019-04-29 07:33:49 UTC
Created attachment 88998 [details] [review]
Bug 22524: Fix publication date and year search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search and year limit in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-
Comment 8 Ere Maijala 2019-04-29 07:34:38 UTC
Mike, I didn't notice the year limit and only fixed the publication date in the normal search terms. Now both should work.
Comment 9 Fridolin SOMERS 2019-04-29 07:44:49 UTC
Bug 19575 also renamed :
date-time-last-modified => date/time-last-modified

Can we fix in this bug ?
Comment 10 Ere Maijala 2019-04-29 07:49:29 UTC
Thanks for pointing that out. I'll add to the patch here.
Comment 11 Ere Maijala 2019-04-29 08:05:10 UTC
Created attachment 88999 [details] [review]
Bug 22524: Fix publication date and year search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search and year limit in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-
Comment 12 Ere Maijala 2019-04-29 08:05:13 UTC
Created attachment 89000 [details] [review]
Bug 22524: Fix date/time-last-modified search with Elasticsearch

Avoid using slash in the field name since it would need to be escaped. Fix conversion of dtlm and any existing mapping.
Comment 13 Michal Denar 2019-04-29 20:01:29 UTC
Hi Ere,
I get error:
Applying: Bug 22524: Fix publication date search with Elasticsearch
Applying: Bug 22524: Fix publication date and year search with Elasticsearch
Using index info to reconstruct a base tree...
M	Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
M	admin/searchengine/elasticsearch/mappings.yaml
M	t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Auto-merging Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
CONFLICT (content): Merge conflict in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
error: Failed to merge in the changes.

Can you rebase it on the top of master. 

Thank you.
Comment 14 Katrin Fischer 2019-04-29 20:28:28 UTC
I thikn the first patch was not obsoleted - can you try again?
Comment 15 Michal Denar 2019-04-29 20:41:40 UTC
Hi Katrin,
still bad:

pplying: Bug 22524: Fix publication date and year search with Elasticsearch
Using index info to reconstruct a base tree...
M	Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
M	admin/searchengine/elasticsearch/mappings.yaml
M	t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t
Auto-merging Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
CONFLICT (content): Merge conflict in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
error: Failed to merge in the changes.
Comment 16 Ere Maijala 2019-04-30 07:48:51 UTC
Katrin, thanks for fixing my mess. The patch applies fine for me.
Comment 17 Liz Rea 2019-04-30 19:33:02 UTC
Created attachment 89152 [details] [review]
Bug 22524: Fix publication date and year search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search and year limit in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 18 Liz Rea 2019-04-30 19:33:12 UTC
Created attachment 89153 [details] [review]
Bug 22524: Fix date/time-last-modified search with Elasticsearch

Avoid using slash in the field name since it would need to be escaped. Fix conversion of dtlm and any existing mapping.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 19 Katrin Fischer 2019-05-11 12:07:36 UTC
I am sorry, but this doesn't work for me. With the patches applied, I find all records for my search for 2012-, ifen records from the 1950s... :(

I have:
- reset mappings
- run updatedatabase
- reset mappings

Maybe I am missing something, please double check the test plan.
Comment 20 Liz Rea 2019-05-13 14:31:58 UTC
It looks like we use both the 008 and the 260 here; I have a record that has 2006 in the 008, but 1995 in the 260 - that record turns up in the search even though I might not initially expect it to - maybe this is what you are seeing? Should we not consider both the 008 and the 260/264?
Comment 21 Liz Rea 2019-05-13 14:45:00 UTC
Hrm, no that's not it because I just found a record in the results after checking more than 30 correct ones that doesn't have a date in the range in any of the supported fields. So yeah, something is messed up here.

Liz
Comment 22 Katrin Fischer 2019-05-13 18:01:22 UTC
(In reply to Liz Rea from comment #21)
> Hrm, no that's not it because I just found a record in the results after
> checking more than 30 correct ones that doesn't have a date in the range in
> any of the supported fields. So yeah, something is messed up here.
> 
> Liz

I think 260/264 is too messy in my opinion for doing numeric seraches. In the past we only used 008 as it has a normalized form of the date. I think that would make sense to me here too.
Comment 23 Liz Rea 2019-05-23 19:06:26 UTC
I think I agree, pub date searches should use the 008, but non-marc libraries coming to Koha might not know how to use the 008 and that... kind of frightens me.
Comment 24 Katrin Fischer 2019-06-02 08:40:26 UTC
(In reply to Liz Rea from comment #23)
> I think I agree, pub date searches should use the 008, but non-marc
> libraries coming to Koha might not know how to use the 008 and that... kind
> of frightens me.

But they might also not add useful data to 260/264...
Comment 25 Nick Clemens 2019-07-16 18:44:24 UTC
Created attachment 91562 [details] [review]
Bug 22524: Fix publication date and year search with Elasticsearch

Also fixes the mappings.yaml to use correct field name (left over issue from bug 19575), so reset mappings and reindex before testing.

Test plan:
1. Reset mappings and reindex biblios.
2. Check that tests in t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t pass.
3. Try that all of the following year range type work in publication date search and year limit in advanced search:
yyyy
yyyy-yyyy
-yyyy
yyyy-

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens 2019-07-16 18:44:28 UTC
Created attachment 91563 [details] [review]
Bug 22524: Fix date/time-last-modified search with Elasticsearch

Avoid using slash in the field name since it would need to be escaped. Fix conversion of dtlm and any existing mapping.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Nick Clemens 2019-07-16 18:48:21 UTC
I think since mappings can be adjusted by libraries we can skirt the issue of which fields should be included - we are finding most libraries want to tweak the mappings rather than use the default file. Needs will vary by library type too - including extra out of the box to show options/functions would be my opinion so this should be good as is.

I couldn't find records that didn't fit the searches in my returns. Can you retest Liz/Katrin and share sample records?

If we don't have examples of incorrect records I would be happy for my SO to be a QA stamp.

The mapping fix here is important so should be split if this one gets stuck
Comment 28 Nick Clemens 2019-07-17 12:35:03 UTC
(In reply to Nick Clemens from comment #27)
> I think since mappings can be adjusted by libraries we can skirt the issue
> of which fields should be included - we are finding most libraries want to
> tweak the mappings rather than use the default file. Needs will vary by
> library type too - including extra out of the box to show options/functions
> would be my opinion so this should be good as is.
> 
> I couldn't find records that didn't fit the searches in my returns. Can you
> retest Liz/Katrin and share sample records?
> 
> If we don't have examples of incorrect records I would be happy for my SO to
> be a QA stamp.
> 
> The mapping fix here is important so should be split if this one gets stuck

Ah, the 260/008 confusion seems to stem from bug 22801 - I think that should depend on this one

Publication date in Adv search dropdown will use pubdate

The 'publication date range' on opac and 'Year' limit on staff both use copydate in ES right now
Comment 29 Nick Clemens 2019-07-26 12:56:56 UTC
Moving to PQA to let RM decide
Comment 30 Martin Renvoize 2019-07-26 15:19:17 UTC
Nice work!

Pushed to master for 19.11.00
Comment 31 Fridolin SOMERS 2019-08-08 10:01:57 UTC
Pushed to 19.05.x for 19.05.03
Comment 32 Lucas Gass 2019-09-03 19:57:47 UTC
backported to 18.11.x for 18.11.10