Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch
Summary: link_bibs_to_authorities.pl doesn't work with Elasticsearch
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All Linux
: P5 - low major (vote)
Assignee: Ere Maijala
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 18825 19893
Blocks:
  Show dependency treegraph
 
Reported: 2017-09-25 11:54 UTC by Frank Hansen
Modified: 2019-10-14 20:02 UTC (History)
11 users (show)

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


Attachments
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch (25.04 KB, patch)
2018-02-01 11:30 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch (1.06 KB, patch)
2018-02-01 15:11 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch (25.53 KB, patch)
2018-03-28 12:22 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (22.04 KB, patch)
2018-09-14 21:23 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (21.08 KB, patch)
2018-09-14 21:25 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (21.07 KB, patch)
2018-09-15 18:51 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.14 KB, patch)
2018-09-16 21:18 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (21.14 KB, patch)
2018-09-16 21:29 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.21 KB, patch)
2018-09-16 21:30 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (20.81 KB, patch)
2018-11-13 14:55 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.21 KB, patch)
2018-11-13 14:55 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.72 KB, patch)
2018-11-13 14:56 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (21.33 KB, patch)
2018-11-13 15:13 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.21 KB, patch)
2018-11-13 15:13 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.72 KB, patch)
2018-11-13 15:13 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: (QA follow-up) Add UNIMARC support (994 bytes, patch)
2018-11-13 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (18.61 KB, patch)
2018-11-13 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.29 KB, patch)
2018-11-13 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.78 KB, patch)
2018-11-13 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (18.60 KB, patch)
2018-11-14 12:03 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.27 KB, patch)
2018-11-14 12:04 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.78 KB, patch)
2018-11-14 12:04 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Clean up, fix and add new tests. (34.04 KB, patch)
2018-11-14 12:04 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (18.61 KB, patch)
2018-11-16 16:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.29 KB, patch)
2018-11-16 16:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.78 KB, patch)
2018-11-16 16:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Clean up, fix and add new tests. (34.09 KB, patch)
2018-11-16 16:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (18.61 KB, patch)
2018-11-16 16:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.29 KB, patch)
2018-11-16 16:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.78 KB, patch)
2018-11-16 16:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Clean up, fix and add new tests. (34.09 KB, patch)
2018-11-16 16:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: (QA follow-up) Ensure we use a modern enough perl (636 bytes, patch)
2018-11-16 16:54 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix several issues with the Elasticsearch code (18.65 KB, patch)
2018-11-20 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Fix search for duplicate authorities with Elasticsearch (2.33 KB, patch)
2018-11-20 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Index authority fields as phrases (29.82 KB, patch)
2018-11-20 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: Clean up, fix and add new tests. (34.14 KB, patch)
2018-11-20 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19365: (QA follow-up) Ensure we use a modern enough perl (633 bytes, patch)
2018-11-20 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Hansen 2017-09-25 11:54:24 UTC
We need to run the script on daily basis to link bibliographic records to matching authority records. We know this worked with Zebra. But with Elasticsearch it don't.

Our current system preference settings is the following:

AutoCreateAuthorites: no. 
BiblioAddsAuthorities: no. 
UseAuthoritiesForTracing: yes. 
CatalogModuleRelink: yes. 
LinkerKeepStale: yes. 
LinkerModule: Default. 
LinkerOptions: broader_headings 
LinkerRelink: yes

In the future we want to AutoCreateAuthorites when matching authority headings does not exist.

The link_bibs_to_authorities.pl runs normally with no errors but no bibliographic records are linked to matching authority records. Here is tha last reports:

Bib authority heading linking report
=======================================================
Linker module:                          C4::Linker::Default
Run started at:                         09/24/17 06:25:04
Run ended at:                           09/25/17 07:53:13
Total run time:                         91688923 ms
Number of bibs checked:                 654037
Number of bibs modified:                0
Number of bibs with errors:             0
Number of headings linked:              0
Number of headings unlinked:            178717
Number of headings fuzzily linked:      1598619
Comment 1 Jonathan Druart 2017-10-04 16:00:31 UTC
This is not blocker, ES is not considered as stable yet.
Comment 2 Ere Maijala 2018-02-01 11:30:15 UTC
Created attachment 71109 [details] [review]
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch

Fix several issues with the Elasticsearch code and optimize it to be usable.
Comment 3 Ere Maijala 2018-02-01 11:33:56 UTC
Attached patch provides a slew of fixes and optimizations to the Elasticsearch code and related linking functionality. 

The optimizations are crucial for a usable process. On my development machine I could only achieve speed of about 1 record per second before the changes. After that it can handle 10-20 modified records and 100 checked records per second. Additionally, avoiding creating a new indexer or searcher every time it's used avoids crashing after a few thousand records due to file handles being exhausted in the Catmandu or Elasticsearch modules.
Comment 4 Ere Maijala 2018-02-01 15:11:50 UTC
Created attachment 71114 [details] [review]
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch

Fix the copyrightdate cast when it's empty.
Comment 5 Nick Clemens 2018-03-12 12:12:10 UTC
Ere, this patch looks good at a first read through, can you provide a test plan for the linker? I am not so familiar with this tool.  

I un-obsoleted the first patch, squashing the followup would be good too
Comment 6 Ere Maijala 2018-03-13 09:47:34 UTC
I have biblios and authorities imported from another ILS so it's a perfect place to test the linking, but it's difficult without proper authority records.


To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Then run "link_bibs_to_authorities.pl -v -l" twice and observe the results. Another option would be to import authorities first e.g. from LoC.
Comment 7 Brendan Gallagher 2018-03-13 13:38:31 UTC
I was able to run my linker with zebra enabled (check)

Switched to Elastic Search in system preferences.  Applied this patch.  Confirmed that I can search in Elastic search.  

Run link_bibs_to_authorities.pl -v -l - failed on first record.
Comment 8 Ere Maijala 2018-03-13 13:52:04 UTC
How did it fail? Did you rebuild the authority index too?
Comment 9 Brendan Gallagher 2018-03-13 14:14:59 UTC
Yes - reindex auths and the linker failed on the first one.
Comment 10 Ere Maijala 2018-03-13 14:18:32 UTC
What's the error? Or did it just not find a match? Did you notice this depends on bug 18825? Without proper mappings it's not going to work correctly.
Comment 11 Brendan Gallagher 2018-03-13 14:35:48 UTC
yes followed all the way through dependencies :) 

here is the error 

Can't locate object method "field" via package "<?xml version="1.0" encoding="UTF-8"?>
<record xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.loc.gov/MARC21/slim" xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd">
Comment 12 Ere Maijala 2018-03-28 11:54:52 UTC
I'm unable to reproduce, but it might be something specific to settings and my environment. Can you find the file and line where the error happens? Thanks!
Comment 13 Ere Maijala 2018-03-28 12:22:38 UTC
Created attachment 73382 [details] [review]
Bug 19365 - link_bibs_to_authorities.pl doesn't work with Elasticsearch

Fix several issues with the Elasticsearch code and optimize it to be usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.
Comment 14 Ere Maijala 2018-03-28 12:23:32 UTC
I rebased the patch on current master. Still couldn't get it to fail.
Comment 15 Nick Clemens 2018-03-28 13:29:31 UTC
It looks like you are touching some of the same code adjusted in bug 19604 - can you take a look there and make any comments and then rebase this on top of that bug?
Comment 16 Ere Maijala 2018-03-28 14:04:56 UTC
Oh, thanks for pointing that one out. I think I'll wait to get some of the fixes into master, this is getting a bit too complicated otherwise!
Comment 17 Nick Clemens 2018-08-15 14:46:41 UTC
This one is cleared to move ahead, any chance of a rebase Ere?
Comment 18 Ere Maijala 2018-08-17 12:12:56 UTC
Yes, I'll see what I can do. I'll probably have to rework this significantly to base this on bug 19893.
Comment 19 Ere Maijala 2018-09-14 21:23:07 UTC
Created attachment 78732 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.
Comment 20 Ere Maijala 2018-09-14 21:25:02 UTC
Created attachment 78737 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.
Comment 21 Brendan Gallagher 2018-09-15 17:44:29 UTC
Talked in person with Ere about the error we found.  trying Z3950 searches failed with something in this patch or with the Depends On
Comment 22 Ere Maijala 2018-09-15 18:51:03 UTC
Created attachment 78875 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.
Comment 23 Ere Maijala 2018-09-15 18:52:06 UTC
I fixed one issue, but I'm not sure if it's related to the Z39.50 problem because I couldn't reproduce it.
Comment 24 Ere Maijala 2018-09-16 21:18:29 UTC
Created attachment 79005 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch
Comment 25 Brendan Gallagher 2018-09-16 21:29:55 UTC
Created attachment 79006 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 26 Brendan Gallagher 2018-09-16 21:30:07 UTC
Created attachment 79007 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 27 Hugo Agud 2018-10-04 14:05:00 UTC
I am not pretty sure if this comment should be here or if it should be a new bug, however link_bibs_to_authorities.pl is working fine, but with new RDA rules, it has one inconvenience.

At PERSONAL_NAMES now there is 100$e - Relator term (R) some libraries have one authoriry for each relator code, like author, translator, editor...etc

link_bibs_ is not looking at $e and matchs all occurrences with the first or last match created and it creates the propertly authority for each $e, but it links wrong


example -->
Authority search results
Results 1 to 6 of 6

Summary	Used in	 
Nombre personal:
Abadal i de Vinyals, Ramon d' , 1888-1970
Details	40 record(s)	
Nombre personal:
Abadal i de Vinyals, Ramon d' , 1888-1970 , ed.
Details	0 records	
Nombre personal:
Abadal i de Vinyals, Ramon d' , 1888-1970 , pr.
Details	0 records
Comment 28 Katrin Fischer 2018-10-04 20:33:15 UTC
(In reply to Hugo Agud from comment #27)
> I am not pretty sure if this comment should be here or if it should be a new
> bug, however link_bibs_to_authorities.pl is working fine, but with new RDA
> rules, it has one inconvenience.
> 
> At PERSONAL_NAMES now there is 100$e - Relator term (R) some libraries have
> one authoriry for each relator code, like author, translator, editor...etc
> 
> link_bibs_ is not looking at $e and matchs all occurrences with the first or
> last match created and it creates the propertly authority for each $e, but
> it links wrong
> 
> 
> example -->
> Authority search results
> Results 1 to 6 of 6
> 
> Summary	Used in	 
> Nombre personal:
> Abadal i de Vinyals, Ramon d' , 1888-1970
> Details	40 record(s)	
> Nombre personal:
> Abadal i de Vinyals, Ramon d' , 1888-1970 , ed.
> Details	0 records	
> Nombre personal:
> Abadal i de Vinyals, Ramon d' , 1888-1970 , pr.
> Details	0 records

I think this should be a separate bug and a separate discussion. We'd certainly not want $e to be taken into account. If it's the same person, should there not only be one authority record? I think someone acting as author and editor would be a common case.
Comment 29 Ere Maijala 2018-10-05 11:28:18 UTC
I agree with Katrin, and we relly need to get all the fixes in and assess the situation once more after that. There are currently some many improvements waiting to make it that it's not worth it to try to manage the dependencies.
Comment 30 Chris Cormack 2018-10-08 22:46:52 UTC
Could you do a rebase please, there we a few too many conflicts for me to fix without breaking things accidentally
Comment 31 Ere Maijala 2018-10-10 09:49:12 UTC
The patch applies cleanly for me, but it requires bug 19893 to be applied first. For some reason git-bz didn't offer to follow there even though it's set as dependency. Anyway, marking BLOCKED until 19893 is done.
Comment 32 Ere Maijala 2018-11-13 14:55:47 UTC
Created attachment 82282 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 33 Ere Maijala 2018-11-13 14:55:53 UTC
Created attachment 82283 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 34 Ere Maijala 2018-11-13 14:56:00 UTC
Created attachment 82284 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).
Comment 35 Ere Maijala 2018-11-13 15:13:31 UTC
Created attachment 82303 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 36 Ere Maijala 2018-11-13 15:13:35 UTC
Created attachment 82304 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 37 Ere Maijala 2018-11-13 15:13:40 UTC
Created attachment 82305 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).
Comment 38 Martin Renvoize 2018-11-13 16:04:32 UTC
Created attachment 82307 [details] [review]
Bug 19365: (QA follow-up) Add UNIMARC support

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize 2018-11-13 16:55:39 UTC
Created attachment 82312 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize 2018-11-13 16:55:44 UTC
Created attachment 82313 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize 2018-11-13 16:55:49 UTC
Created attachment 82314 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize 2018-11-13 17:07:01 UTC
We've fallen at the last hurdle here I'm afraid.. we need some updated/additional unit tests to cover the changes.. at a minimum Koha:: namespace changes, but I'd really love to see some minimal coverage of C4::Biblio added for the affected method too...
Comment 43 Ere Maijala 2018-11-14 12:03:55 UTC
Created attachment 82325 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Ere Maijala 2018-11-14 12:04:01 UTC
Created attachment 82326 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Ere Maijala 2018-11-14 12:04:07 UTC
Created attachment 82327 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Ere Maijala 2018-11-14 12:04:13 UTC
Created attachment 82328 [details] [review]
Bug 19365: Clean up, fix and add new tests.

Moved db_dependent tests to the proper directory in the hierarchy, fixed tests to check correct results now that the authority query builder works better and added several new tests to cover changed functionality.
Comment 47 Martin Renvoize 2018-11-16 16:40:55 UTC
Created attachment 82406 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2018-11-16 16:41:01 UTC
Created attachment 82407 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2018-11-16 16:41:07 UTC
Created attachment 82408 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2018-11-16 16:41:13 UTC
Created attachment 82409 [details] [review]
Bug 19365: Clean up, fix and add new tests.

Moved db_dependent tests to the proper directory in the hierarchy, fixed tests to check correct results now that the authority query builder works better and added several new tests to cover changed functionality.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2018-11-16 16:54:10 UTC
Created attachment 82410 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2018-11-16 16:54:16 UTC
Created attachment 82411 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2018-11-16 16:54:21 UTC
Created attachment 82412 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2018-11-16 16:54:27 UTC
Created attachment 82413 [details] [review]
Bug 19365: Clean up, fix and add new tests.

Moved db_dependent tests to the proper directory in the hierarchy, fixed tests to check correct results now that the authority query builder works better and added several new tests to cover changed functionality.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2018-11-16 16:54:33 UTC
Created attachment 82414 [details] [review]
Bug 19365: (QA follow-up) Ensure we use a modern enough perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 56 Martin Renvoize 2018-11-16 16:56:28 UTC
Tests all pass, QA script is now happy and I can't find any regressions.

It took a bit of digging through to check we hadn't inadvertently lost ny tests, but the cleanup was worthwhile.

Had to add a minor followup to declare perl `use 5.010` to ensure the 'state' variables are available for compile and squash a qa script warning.

Passing QA
Comment 57 Martin Renvoize 2018-11-20 10:07:27 UTC
Created attachment 82530 [details] [review]
Bug 19365: Fix several issues with the Elasticsearch code

Also optimize it so it's actually usable.

Test plan:

1. To test it properly you need biblio and authority data. You might get away with enabling AutoCreateAuthorities and BiblioAddsAuthorities so that authorities are created in the process. Another option would be to import authorities first e.g. from LoC.
2. Make sure the authority index has been properly created with "misc/search_tools/rebuild_elastic_search.pl -a -d"
3. Run "misc/link_bibs_to_authorities.pl -v -l" twice and observe the results.

Sponsored-by: National Library of Finland
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 58 Martin Renvoize 2018-11-20 10:07:33 UTC
Created attachment 82531 [details] [review]
Bug 19365: Fix search for duplicate authorities with Elasticsearch

Sponsored-by: National Library of Finland
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 59 Martin Renvoize 2018-11-20 10:07:39 UTC
Created attachment 82532 [details] [review]
Bug 19365: Index authority fields as phrases

Followup change to the mappings syntax from bug 19893. Indexing as phrases allows to properly search for matching headings comprised of multiple subfields (e.g. author name with year of birth).

Sponsored-by: National Library of Finland
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 60 Martin Renvoize 2018-11-20 10:07:44 UTC
Created attachment 82533 [details] [review]
Bug 19365: Clean up, fix and add new tests.

Moved db_dependent tests to the proper directory in the hierarchy, fixed tests to check correct results now that the authority query builder works better and added several new tests to cover changed functionality.

Sponsored-by: National Library of Finland
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 61 Martin Renvoize 2018-11-20 10:07:50 UTC
Created attachment 82534 [details] [review]
Bug 19365: (QA follow-up) Ensure we use a modern enough perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 62 Ere Maijala 2018-11-26 12:03:29 UTC
I removed dependency on 21826 since it's not absolutely really required for the fixes here.
Comment 63 Nick Clemens 2018-11-26 16:15:01 UTC
Awesome work all!

Pushed to master for 18.11
Comment 64 Jesse Maseto 2018-12-13 20:47:14 UTC
Will need patches rebase for 18.05, if needed for backporting to 18.05.