Bug 30143 - OAI-PMH provider may end up in an eternal loop due to missing sort
Summary: OAI-PMH provider may end up in an eternal loop due to missing sort
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Ere Maijala
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 29135
Blocks:
  Show dependency treegraph
 
Reported: 2022-02-21 13:49 UTC by Ere Maijala
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact: Martin Renvoize
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00


Attachments
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records (921 bytes, patch)
2022-02-21 14:10 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records (1.04 KB, patch)
2022-04-23 19:31 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records (1.14 KB, patch)
2022-04-29 08:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ere Maijala 2022-02-21 13:49:26 UTC
The OAI-PMH provider may end up in an eternal loop when processing deleted records due to the select statement missing sort (and limit). If records are not returned from the database in biblionumber order naturally, this omission may lead to the same records to be processed with each subsequent request, or records to be skipped.

Fix coming up.
Comment 1 Ere Maijala 2022-02-21 13:51:29 UTC
Note: this can only happen when include_items is enabled in oai.conf, which contributes to it lurking around as long as it has.
Comment 2 Ere Maijala 2022-02-21 14:10:09 UTC
Created attachment 130961 [details] [review]
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records

The query was missing the "ORDER BY" and "LIMIT" clauses, which could make retrieve a wrong set of records.
Comment 3 Ere Maijala 2022-02-21 14:12:22 UTC
I can't provide steps to reproduce since my database always returns the records in biblionumber order when querying without a specific ORDER BY clause. This issue was, however, encountered in a production system. The mistake originates from refactoring done in bug 29135.
Comment 4 Joonas Kylmälä 2022-04-23 19:27:58 UTC
Depends on bug 29135. The fix is correct, however maybe it would make to sense to move out the common SQL above the if-else condition like it was before bug 29135, this way we wouldn't accidentally forget to change this SQL query in both places in the future. Not a blocker for this, gonna add my sign-off.
Comment 5 Joonas Kylmälä 2022-04-23 19:31:54 UTC
Created attachment 133703 [details] [review]
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records

The query was missing the "ORDER BY" and "LIMIT" clauses, which could make retrieve a wrong set of records.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 6 Marcel de Rooy 2022-04-29 08:58:30 UTC
Created attachment 134352 [details] [review]
Bug 30143: OAI-PMH: Fix SQL query used to fetch deleted records

The query was missing the "ORDER BY" and "LIMIT" clauses, which could make retrieve a wrong set of records.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Fridolin Somers 2022-05-03 21:27:40 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 8 Victor Grousset/tuxayo 2022-06-23 02:01:08 UTC
Note: it has been backported to 21.11.x
Comment 9 Victor Grousset/tuxayo 2022-06-23 02:01:21 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.
Comment 10 Martin Renvoize 2022-06-23 13:45:48 UTC
Architectural fix, no documentation change required.