[Bug 33554] New: searching borrowers is a lot slower if there's searchable extended attributes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Bug ID: 33554 Summary: searching borrowers is a lot slower if there's searchable extended attributes. Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: didier.gautheron@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Depends on: 30055 Hi, Since BZ 30055 search is using the REST API. For a library with 80,000 borrowers and 800,000 borrower_attributes, (one searchable per borrower), searching a borrower is twice as slow and is noticeable in checkin/checkout. before 30055 the request was something like : SELECT borrowernumber FROM borrowers WHERE firstname` LIKE '%foo%' OR middle_name LIKE '%foo%' OR surname LIKE '%foo%' OR othernames LIKE '%foo%' OR cardnumber LIKE '%foo%' OR userid LIKE '%foo%' OR borrowernumber in ( select borrowernumber from borrower_attributes where attribute LIKE '%foo%' AND code = 'BAR' ) GROUP BY borrowernumber; runtime 0.069 sec in master SELECT borrowernumber FROM borrowers LEFT JOIN borrower_attributes USING(borrowernumber) WHERE firstname` LIKE '%foo%' OR middle_name LIKE '%foo%' OR surname LIKE '%foo%' OR othernames LIKE '%foo%' OR cardnumber LIKE '%foo%' OR userid LIKE '%foo%' OR (attribute LIKE '%foo%' AND code = 'BAR' ) GROUP BY borrowernumber; runtime 1.324 sec Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30055 [Bug 30055] Rewrite some of the patron searches to make them use the REST API routes -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33489 CC| |nick@bywatersolutions.com --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- An index on those tables might help - we saw similar issues on a site with many borrowers. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #2 from Didier Gautheron <didier.gautheron@biblibre.com> --- (In reply to Nick Clemens from comment #1)
An index on those tables might help - we saw similar issues on a site with many borrowers.
In my understanding these tables are already fully indexed. I did play with indexes' statistics but got no improvement. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Would it be reasonable if making an attribute searchable didn't mean it was searched by default? i.e. we could add the attribute to the search dropdowns, or have an option on the attribute to include in searches by default -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I was going to suggest to have a separate "extended_attributes" parameter we could pass to the endpoint, but that's not trivial to implement (we need the search_type, and to know if we need a OR or AND with q...) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #4)
I was going to suggest to have a separate "extended_attributes" parameter we could pass to the endpoint, but that's not trivial to implement (we need the search_type, and to know if we need a OR or AND with q...)
what about storing searchable attribute values in a new borrowers column? Any time a searchable attribute is modified, we would just regenerate the text for that column. Then we would no longer need to join on the attributes tables. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #6 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Jonathan Druart from comment #4)
I was going to suggest to have a separate "extended_attributes" parameter we could pass to the endpoint, but that's not trivial to implement (we need the search_type, and to know if we need a OR or AND with q...)
I think you'd need to pass all three parameters (the search text, the search field, and the search type) to make it work. Would that be acceptable to have this special case inside Koha::REST::V1::Patrons::list ? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Tomas, any ideas how to fix this correctly? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I've asked my DBA to analyze it. Nick and I have talked about making a specialized endpoint that restores the subquery, but this might be solvable with some DB optimizations. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Julian Maurice from comment #6)
(In reply to Jonathan Druart from comment #4)
I was going to suggest to have a separate "extended_attributes" parameter we could pass to the endpoint, but that's not trivial to implement (we need the search_type, and to know if we need a OR or AND with q...)
I think you'd need to pass all three parameters (the search text, the search field, and the search type) to make it work. Would that be acceptable to have this special case inside Koha::REST::V1::Patrons::list ?
Should we do it on a specialized endpoint? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- I am working on a 'patrons/lookup' endpoint to see about building the query in a more performant way. For testing, I imported 100k borrowers with several attributes each and was able to confirm the slowdown when attributes are enabled I dumped the tables and made them available on github: https://github.com/kidclamp/sample_files/blob/main/many_attributes.gz https://github.com/kidclamp/sample_files/blob/main/many_borrowers.gz -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 154331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154331&action=edit Bug 33554: [WIP] Add a new lookup endpoint for patron searching This patch adds a new API lookup optoin for patrons. This endpoint takes three params: search_type - contains or starts with search_term - the string passed to search search_field - a comma separated string of fields to search The patron-search code is altered to pass these additional params, and the datatables code is updated to pass these params through The lookup code parses the attributes search directly, then adds it as a subquery to the results This should only affect patron/checkout searches - other patron searches are not changed at this time NOTE: In testing I noted that a search for a patron field plus an attribute returns nothing, but this is the same as existing code. i.e. patron Kenneth ABRAMS with SCHOOL attribute 'Oxford' is not returned for 'ken oxf' search WIP: Needs test coverage TO test: 1 - Add the sample borrowers and attributes to your DB 2 - Perform some patron searches 'ken' 'ken abr' 'oxford' 'ken oxford' 3 - Note the response times, most are slow 4 - Apply patch, restart all 5 - Repeat searches 6 - Note faster response times, but same results -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #12 from Kyle M Hall <kyle@bywatersolutions.com> --- I feel like we could greatly simplify and speed up patron searching by simply have a new table column or columns that exist solely to contain searchable content. We could even store this in JSON format and make use of JSON functions at a later date if possible. Here are the default patron search fields: firstname,middle_name,surname,othernames,cardnumber,userid Let's say I have a patron: Joe Gerald Patronson, esquire ( 123456789 ), jpatronson Upon editing, we could store the following: { firstname: "Joe", middle_name: "Gerald", surname: "Patronson", othernames: "esquire", cardnumber: "123456789", userid: "jpatronson", } Now, if we need to search the entire field we can do a LIKE ': "%<VALUE>%\n' for contains and LIKE ': "<VALUE>%\n' for begins with If we want to search for patrons with a firstname starting with J and surname containing son it would like like WHERE newcolumn LIKE 'firstname: "J%"\n' AND newcolumn LIKE 'surname: "%son%"\n' If we do this, we can have a single column to search and index. We can also add extended attributes to this by attribute code, for example: { firstname: "Joe", middle_name: "Gerald", surname: "Patronson", othernames: "esquire", cardnumber: "123456789", userid: "jpatronson", ATTR: "value1", ATTR: "value2", } Right now we cannot search on specific attributes, but that could easily be added using this system. This allows us to search all needed patron data without the need for complex joins. I think TINYTEXT might be a bit too small, but we could definitely be fine with TEXT and we can specify the length of the index as a reasonable number ( say 500 characters or so ). Thoughts? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- We can also resurrect bug 17500... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 koha-US bug tracker <bugzilla@koha-us.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bugzilla@koha-us.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #13)
We can also resurrect bug 17500...
I think that's the best idea. It's gonna take time though, assuming you are interesting in picking it back up. If you are I'd be happy to help how I can! In the mean time we can try to get a quick fix that gets us to acceptable. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- If this approach looks reasonable then I will write tests, and update to fix the attribute search. Just let me know if I should put this time in -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30645 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33428 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wizzyrea@gmail.com --- Comment #16 from Liz Rea <wizzyrea@gmail.com> --- (In reply to Nick Clemens from comment #15)
If this approach looks reasonable then I will write tests, and update to fix the attribute search. Just let me know if I should put this time in
To my mind, we need a fix here sooner than later. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=34517 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Didier, Julian, can you have a look at this patch and test it? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #18 from Didier Gautheron <didier.gautheron@biblibre.com> --- (In reply to Jonathan Druart from comment #17)
Didier, Julian, can you have a look at this patch and test it?
For real life test we only have < 22.11.06 customers and patch doesn't apply. In my understanding it applies on to be released 22.11.10, a customer will upgrade next month and I can test it then. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Failed QA --- Comment #19 from Julian Maurice <julian.maurice@biblibre.com> --- The patch breaks patrons search for me. If I use the top search bar it seems to work, but after that, using the form on the left doesn't do anything. If I go to patrons search by using the navigation link, the letters don't work. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 --- Comment #20 from Julian Maurice <julian.maurice@biblibre.com> --- Comment on attachment 154331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154331 Bug 33554: [WIP] Add a new lookup endpoint for patron searching Review of attachment 154331: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33554&attachment=154331) ----------------------------------------------------------------- ::: Koha/Patrons.pm @@ +93,5 @@
+ } + $attributes_rs = Koha::Patron::Attributes->search( { -and => \@attributes_query } ); + } + + push @{ $query->{-or} }, { 'me.borrowernumber' => { -in => [ $attributes_rs->get_column('borrowernumber') ] } };
This will break if ExtendedPatronAttributes is disabled ($attributes_rs will be undef) Also, if no patron attributes are searchable, `$attributes_rs->get_column('borrowernumber')` will return borrowernumbers of all borrowers that have at least one attribute. It's probably a good idea to use `$attributes_rs->_resultset->get_column('borrowernumber')->as_query` instead in order to not have to fetch all borrowernumbers first (not tested) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We should generalise this into Koha::REST::Plugin::Query.. that way we could get the performance boost for illattributes, extended_attributes and borrower_attributes who all follow the same patturn. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36025 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 sbarter@aacpl.net changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sbarter@aacpl.net --- Comment #22 from sbarter@aacpl.net --- I use a searchable extended attribute as part of a workflow for moving patrons into different patron lists. Having the attribute available as a choice in the Search field drop-down would work well. We have over 277,000 patrons so this does have quite an impact. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Mengu Yazicioglu <mengu@devinim.com.tr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mengu@devinim.com.tr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|searching borrowers is a |Searching borrowers is a |lot slower if there's |lot slower if there's |searchable extended |searchable extended |attributes. |attributes CC| |david@davidnind.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33554 Mathieu Saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org