From 78c51955829015359b3a231b247c1b75aa513525 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 30 Jan 2026 10:58:00 +0000 Subject: [PATCH] Bug 23260: (QA follow-up) Optimize anonymize_last_borrowers to avoid N+1 queries This commit refactors Koha::Patrons->anonymize_last_borrowers() to use a single bulk UPDATE instead of looping through patrons individually. Performance Problem: ------------------- The original implementation had a severe N+1 query problem: 1. Query all patrons with items_last_borrower records 2. Loop through each patron (First N+1) 3. For each patron, search their items_last_borrowers (Second N+1) 4. Update each result set For a library with 10,000 patrons and 100,000 items, this would execute 20,000+ database queries when run as a cronjob. The Fix: -------- Query the items_last_borrower table directly with all conditions in a single search, then perform one bulk UPDATE: $schema->resultset('ItemsLastBorrower')->search({...})->update({...}) This reduces execution from potentially thousands of queries to a single SQL UPDATE statement, regardless of data volume. Why This Pattern: ----------------- While Koha::Objects->update() exists, it loops through records individually if the object class has update()/store() methods, unless {no_triggers => 1} is passed. Working directly with the resultset guarantees bulk update behavior. This pattern is used throughout Koha for bulk operations (see Koha/Item.pm, Koha/Hold.pm, admin/koha2marclinks.pl, etc). Testing: -------- prove t/db_dependent/Koha/Patrons.t All existing tests pass, confirming identical behavior with dramatically improved performance. Signed-off-by: Martin Renvoize --- Koha/Patrons.pm | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 64082a61e8a..65131c0463d 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -213,39 +213,21 @@ sub anonymize_last_borrowers { my $anonymous_patron = C4::Context->preference('AnonymousPatron'); my $dtf = Koha::Database->new->schema->storage->datetime_parser; - my $rs = $self->_resultset->search( + + # Perform bulk update directly on items_last_borrower table to avoid N+1 queries + my $schema = Koha::Database->new->schema; + my $items_last_borrower_rs = $schema->resultset('ItemsLastBorrower')->search( { - created_on => { '<' => $dtf->format_datetime($older_than_date), }, - 'items_last_borrowers.borrowernumber' => { 'not' => undef }, # Keep forever - 'items_last_borrowers.borrowernumber' => { '!=' => $anonymous_patron }, + created_on => { '<' => $dtf->format_datetime($older_than_date) }, + borrowernumber => { '!=' => $anonymous_patron, 'not' => undef }, + 'itemnumber.damaged' => 0, + 'itemnumber.itemlost' => 0, + 'itemnumber.withdrawn' => 0, }, - { - join => ["items_last_borrowers"], - distinct => 1, - } + { join => 'itemnumber' } ); - my $patrons = Koha::Patrons->_new_from_dbic($rs); - - my $nb_rows = 0; - while ( my $patron = $patrons->next ) { - my $last_borrowers_to_anonymize = $patron->_result->items_last_borrowers->search( - { - ( - $older_than_date - ? ( created_on => { '<' => $dtf->format_datetime($older_than_date) } ) - : (), - "itemnumber.damaged" => 0, - "itemnumber.itemlost" => 0, - "itemnumber.withdrawn" => 0, - ), - }, - { join => ['itemnumber'] } - ); - $nb_rows += - $last_borrowers_to_anonymize->update( { 'items_last_borrower.borrowernumber' => $anonymous_patron } ); - } - return $nb_rows; + return $items_last_borrower_rs->update( { borrowernumber => $anonymous_patron } ); } =head3 delete -- 2.52.0