From 5808d6ec73212af5efa26011f86a4feb45680a4d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 9 Mar 2026 10:12:22 +0000 Subject: [PATCH] Bug 41801: (QA follow-up) Replace MAX subquery with in-memory array count for rank clamping The lowestPriority rank-clamping block fired a second SQL query to get max(priority) WHERE lowestPriority=0 and used that value as both a threshold and a splice index. This was fragile: if priorities had become discontiguous (e.g. due to a race or a bug elsewhere), the DB priority value would not match the 1-based position in @priority, and the splice could land the hold in the wrong slot. The @priority array already contains all the information needed: it is sorted lowestPriority ASC so the count of rows where lowestPriority=0 is exactly the number of non-lowest holds, which equals the last valid rank for a normal hold. Fetch lowestPriority in the existing SELECT and use scalar grep { !$_->{lowestPriority} } @priority instead. This eliminates the extra DB round-trip and makes the clamping operate on the same data snapshot that will be written back. --- C4/Reserves.pm | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 63760e3a9bf..fe555348490 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1703,7 +1703,7 @@ sub FixPriority { # get what's left, sorting lowestPriority holds to the bottom my $query = " - SELECT reserve_id, borrowernumber, reservedate + SELECT reserve_id, borrowernumber, reservedate, lowestPriority FROM reserves WHERE biblionumber = ? AND ((found <> 'W' AND found <> 'T' AND found <> 'P') OR found IS NULL) @@ -1726,20 +1726,12 @@ sub FixPriority { } } - # if this hold is marked lowest priority, we can only move it so far + # if this hold is marked lowest priority, we can only move it so far; + # cap rank to just after the last non-lowestPriority hold using the + # already-fetched @priority array (avoids a second DB query and stale data) if ( $hold && $hold->lowestPriority && $rank ne 'del' && $rank > 0 ) { - my $query = " - SELECT max(priority) - FROM reserves - WHERE biblionumber = ? - AND ((found <> 'W' AND found <> 'T' AND found <> 'P') OR found IS NULL) - AND lowestPriority = 0; - "; - my $sth = $dbh->prepare($query); - $sth->execute($biblionumber); - my ($highest_non_lowest_priority) = $sth->fetchrow_array(); - $rank = $highest_non_lowest_priority + 1 - if ( $highest_non_lowest_priority && $rank <= $highest_non_lowest_priority ); + my $non_lowest_count = scalar grep { !$_->{lowestPriority} } @priority; + $rank = $non_lowest_count + 1 if $non_lowest_count && $rank <= $non_lowest_count; } # if index exists in array then move it to new position -- 2.53.0