Summary: | Search for patron '%' returns all results | ||
---|---|---|---|
Product: | Koha | Reporter: | Chris Cormack <chris> |
Component: | Patrons | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | boutrosboutrosboutros, jonathan.druart, nick, oleonard, veron |
Version: | Main | Keywords: | Academy |
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30055 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 6253 | ||
Attachments: |
Bug 1707: Do not return more than 100 patrons matching the attributes search
Bug 1707: Do not return more than 100 patrons matching the attributes search Bug 1707: Do not return more than 100 patrons matching the attributes search Bug 1707 (QA Followup) Add a warning that attribute searches will be truncated |
Description
Chris Cormack
2010-05-21 00:33:40 UTC
This is still a valid bug. Is this still a problem with the new server side processing of the patron search table? This is still a problem. I'd say the member search is dangerous and generates some horrible queries which takes many minutes to run if you have a large dataset. We have over 300,000 patrons. If someone search for something stupid like %29% - this is (part of) the generated query: https://gist.githubusercontent.com/boutros/db14414591ea77638b2a3a183a18ec00/raw/d59f7ef33890b7a034f24a0d350b5af3b6257e4e/bigquery.sql (In reply to Petter Goksøyr Åsen from comment #3) > This is still a problem. I'd say the member search is dangerous and > generates some horrible queries which takes many minutes to run if you have > a large dataset. > > We have over 300,000 patrons. > > If someone search for something stupid like %29% - this is (part of) the > generated query: > https://gist.githubusercontent.com/boutros/db14414591ea77638b2a3a183a18ec00/ > raw/d59f7ef33890b7a034f24a0d350b5af3b6257e4e/bigquery.sql How d(In reply to Petter Goksøyr Åsen from comment #3) > This is still a problem. I'd say the member search is dangerous and > generates some horrible queries which takes many minutes to run if you have > a large dataset. > > We have over 300,000 patrons. > > If someone search for something stupid like %29% - this is (part of) the > generated query: > https://gist.githubusercontent.com/boutros/db14414591ea77638b2a3a183a18ec00/ > raw/d59f7ef33890b7a034f24a0d350b5af3b6257e4e/bigquery.sql For the records: http://irc.koha-community.org/koha/2016-10-13#i_1866950 Petter, how did you finally fix it? I have several ways to fix it but I don't figure out which one is the best. I have put a limit on the query as you suggested: http://paste.koha-community.org/135 It's not in production yet, but testing in staging so far seems good. The result is probably not accurate, as you said, but I think it's only potentially wrong if you have a very wide search (i.e few numbers which matches lots of borrowernumbers). If the result list is long, say tens of thousands, it does not matter that its not accurate. If you search for full fields (in our case its social security number as extended patron attribute), the returned result is always correct In my mind, it's quite dangerous that Koha allows to generate SQL-queries which are several MB of text, which generated huge loads on our server, so I'd prefer safety to accuracy here. Created attachment 57068 [details] [review] Bug 1707: Do not return more than 100 patrons matching the attributes search As you can feel, this patch is not perfect as we are hardcodind the number of results to return. But this number should be correct in the most cases. Test plan: With a large number of patrons, using attributes, you can search for something very generic, like '%'. Without this patch, the MySQL server could be overloaded because of the generated query (built from borrowernumbers). Comment on attachment 57068 [details] [review] Bug 1707: Do not return more than 100 patrons matching the attributes search Review of attachment 57068 [details] [review]: ----------------------------------------------------------------- ::: C4/Members/Attributes.pm @@ +152,4 @@ > JOIN borrower_attribute_types USING (code) > WHERE staff_searchable = 1 > AND (} . join (" OR ", map "attribute like ?", @$filter) .qq{)}; > + $query .= q{ LIMIT ?} if $limit; LIMIT is a MySQLism. Perhaps DBIx would be a good way to get around this? Not that I know how to do it, but this looks like a good place to start: http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/ResultSet.pm#rows (In reply to M. Tompsett from comment #7) > LIMIT is a MySQLism. Hum? Ref needed please. https://www.postgresql.org/docs/current/static/queries-limit.html (In reply to M. Tompsett from comment #7) > LIMIT is a MySQLism. If this page is accurate, LIMIT is not standard but is not unique to MySQL: http://troels.arvin.dk/db/rdbms/#select-limit IMO there is a LIMIT to how much database-agnosticism we should worry about. Created attachment 62301 [details] [review] Bug 1707: Do not return more than 100 patrons matching the attributes search As you can feel, this patch is not perfect as we are hardcodind the number of results to return. But this number should be correct in the most cases. Test plan: With a large number of patrons, using attributes, you can search for something very generic, like '%'. Without this patch, the MySQL server could be overloaded because of the generated query (built from borrowernumbers). Created attachment 62324 [details] [review] Bug 1707: Do not return more than 100 patrons matching the attributes search As you can feel, this patch is not perfect as we are hardcodind the number of results to return. But this number should be correct in the most cases. Test plan: With a large number of patrons, using attributes, you can search for something very generic, like '%'. Without this patch, the MySQL server could be overloaded because of the generated query (built from borrowernumbers). Signed-off-by: Marc Véron <veron@veron.ch> I think this fixes a problem, but I'd really like to see a notification/error pushed back to the user letting them know the search was truncated and suggesting a more exact search This feels similar to bug 15812 We cannot really know the search has been truncated. Created attachment 63335 [details] [review] Bug 1707 (QA Followup) Add a warning that attribute searches will be truncated (In reply to Jonathan Druart from comment #13) > We cannot really know the search has been truncated. True, but in that case we can just always warn - if we are going to have a hardcoded limit it just needs to be announced (In reply to Nick Clemens from comment #15) > (In reply to Jonathan Druart from comment #13) > > We cannot really know the search has been truncated. > > True, but in that case we can just always warn - if we are going to have a > hardcoded limit it just needs to be announced Yes but the note is wrong, and will be hard to make it correct :) Note you added says "Note: Patron attribute searching is enabled, to avoid memory issues a limit of 100 patrons will be returned during "standard" searches" If you are searching for a pattern that is used 250x in patron attribute, and 100x in other standard fields, we will certainly get less 100 results even if the whole set would have been 100. Could we close this with the "new" API based patron search? (In reply to Katrin Fischer from comment #17) > Could we close this with the "new" API based patron search? +1 |