Empty patron search can lead to heavy DB query / client error

Bug #1465847 reported by Bill Erickson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Undecided
Unassigned
2.7
Undecided
Unassigned
2.8
Undecided
Unassigned

Bug Description

Confirmed in Evergreen master (circa 2.8) and 2.5.

Clicking "Search" in the XUL client patron search with no search parameters results in a DB query which basically amounts to "Give me all patrons, sorted by name (etc.), limit to 50". On a large data set, this is a heavy query.

(The browser client does not have the same problem, because of how it compiles patron searches. Its empty queries are empty enough for the API to quickly exit).

In the end, it's not clear if this is a feature or a bug. Given the dubious value of such a query (and the "patron search is not a reporting tool" refrain), I'm inclined to consider this a bug.

Should we require the API to expect at least one filter?

To further guide staff toward sane queries, do we disable the patron Search button when no search values are present?

Revision history for this message
Bill Erickson (berick) wrote :

The "client error" I reference in the summary is the usual "A script on this page may be busy..." XUL client error.

Revision history for this message
Bill Erickson (berick) wrote :

Pushed a branch which applies sanity checks to patron search API params. If all search parameters are empty (zero or more spaces) or invalid, API call exits early with no results.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1465847-patron-search-require-filter

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.8.3
milestone: 2.8.3 → 2.9-alpha
Changed in evergreen:
milestone: 2.9-alpha → 2.9-beta
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

While I was not able to reproduce the bug, the code does what it says on the tin.

I feel that I should add that neither changing the search location nor checking the box to include inactive patrons counts as adding a filter. You have to fill in one of the search criteria text boxes in order to complete a search.

I'll leave this here for a day or two so there can be discussion if desired. Otherwise, I think we should merger this by next Wednesday's releases.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Changed in evergreen:
milestone: 2.9-beta → 2.9.0
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Well, it is past the time that I originally said we should push this, so I tested it again and pushed it to master, rel_2_8, and rel_2_7.

Thanks, Bill!

Changed in evergreen:
status: New → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers