Bug 39920 - do_check_for_previous_checkout should us 'IN' over 'OR'
Summary: do_check_for_previous_checkout should us 'IN' over 'OR'
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords: no-sandbox
Depends on:
Blocks:
 
Reported: 2025-05-16 12:50 UTC by Nick Clemens (kidclamp)
Modified: 2025-06-19 09:01 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.01,24.11.06
Circulation function:


Attachments
Bug 39920: Update query for check previous checkout to use IN (1.16 KB, patch)
2025-05-16 12:53 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 39920: Update query for check previous checkout to use IN (1.20 KB, patch)
2025-05-17 19:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 39920: Update query for check previous checkout to use IN (1.26 KB, patch)
2025-05-22 16:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2025-05-16 12:50:46 UTC
When forming the query for previous checkout we have:
 876     # Create (old)issues search criteria
 877     my $criteria = {
 878         borrowernumber => $self->borrowernumber,
 879         itemnumber     => \@item_nos,
 880     };

This forms a query like:
SELECT COUNT( * ) FROM `issues` `me` WHERE ( ( `borrowernumber` = ? AND ( `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? ) ) ):


If we instead do:
itemnumber => { -in => \@item_nos }

We get a query like:
SELECT COUNT( * ) FROM `issues` `me` WHERE ( ( `borrowernumber` = ? AND `itemnumber` IN ( ?, ?, ?, ? ) ) )

I believe in the past we fond that MySQL has optimizations for IN and it shortens the query in logs
Comment 1 Nick Clemens (kidclamp) 2025-05-16 12:53:29 UTC
Created attachment 182528 [details] [review]
Bug 39920: Update query for check previous checkout to use IN

This patch simply alters the query fro checkign previous checkouts to use
IN instead of a list of OR for itemnumbers

To test:
1 - On the command line:
    export DBICE_TRACE=1
2 - perl -e 'use Koha::Patrons;my $patron = Koha::Patrons->find(5);my $item = Koha::Items->find(54);warn $item->itemnumber;$patron->do_check_for_previous_checkout( $item->unblessed );'
3 - Note the query
4 - Apply patch
5 - Repeat 2
6 - Note the shorter query
Comment 2 David Nind 2025-05-17 19:13:57 UTC
Created attachment 182563 [details] [review]
Bug 39920: Update query for check previous checkout to use IN

This patch alters the query for checking previous checkouts to use
IN instead of a list of OR for itemnumbers.

To test:
1 - On the command line:
    export DBIC_TRACE=1
2 - perl -e 'use Koha::Patrons;my $patron = Koha::Patrons->find(5);my $item = Koha::Items->find(54);warn $item->itemnumber;$patron->do_check_for_previous_checkout( $item->unblessed );'
3 - Note the query
4 - Apply patch
5 - Repeat 2
6 - Note the shorter query

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2025-05-17 19:18:33 UTC
Testing notes (using KTD):

1. For step 1, it should be export DBIC_TRACE=1 (not export DBICE_TRACE=1) (I amended the patch commit message).

2. Output before the patch:

...
SELECT COUNT( * ) FROM `issues` `me` WHERE ( ( `borrowernumber` = ? AND ( `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? ) ) ): '5', '52', '53', '54', '55'
SELECT COUNT( * ) FROM `old_issues` `me` WHERE ( ( `borrowernumber` = ? AND ( `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? OR `itemnumber` = ? ) ) ): '5', '52', '53', '54', '55'

3. Output after the patch:

...
SELECT COUNT( * ) FROM `issues` `me` WHERE ( ( `borrowernumber` = ? AND `itemnumber` IN ( ?, ?, ?, ? ) ) ): '5', '52', '53', '54', '55'
SELECT `me`.`variable`, `me`.`value`, `me`.`options`, `me`.`explanation`, `me`.`type` FROM `systempreferences` `me` WHERE ( `me`.`variable` = ? ): 'checkprevcheckoutdelay'
SELECT COUNT( * ) FROM `old_issues` `me` WHERE ( ( `borrowernumber` = ? AND `itemnumber` IN ( ?, ?, ?, ? ) ) ): '5', '52', '53', '54', '55'
Comment 4 Martin Renvoize (ashimema) 2025-05-22 16:12:07 UTC
Created attachment 182744 [details] [review]
Bug 39920: Update query for check previous checkout to use IN

This patch alters the query for checking previous checkouts to use
IN instead of a list of OR for itemnumbers.

To test:
1 - On the command line:
    export DBIC_TRACE=1
2 - perl -e 'use Koha::Patrons;my $patron = Koha::Patrons->find(5);my $item = Koha::Items->find(54);warn $item->itemnumber;$patron->do_check_for_previous_checkout( $item->unblessed );'
3 - Note the query
4 - Apply patch
5 - Repeat 2
6 - Note the shorter query

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 5 Lucas Gass (lukeg) 2025-06-02 20:59:11 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 6 Fridolin Somers 2025-06-18 12:57:16 UTC
I see in 25.05.x
Comment 7 Fridolin Somers 2025-06-18 12:58:00 UTC
Pushed to 24.11.x for 24.11.06
Comment 8 Paul Derscheid 2025-06-19 09:01:59 UTC
Nice work everyone!

Pushed to 25.05.x for 25.05.03