Bugzilla – Attachment 192494 Details for
Bug 28530
Allow configuration of floating limits by item type
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28530: (QA follow-up) Fix N+1 query performance issue
53c1d23.patch (text/plain), 10.48 KB, created by
Martin Renvoize (ashimema)
on 2026-02-04 18:38:57 UTC
(
hide
)
Description:
Bug 28530: (QA follow-up) Fix N+1 query performance issue
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-04 18:38:57 UTC
Size:
10.48 KB
patch
obsolete
>From 53c1d23069db84a23563c7eb5a634a1772097c32 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Wed, 4 Feb 2026 17:55:15 +0000 >Subject: [PATCH] Bug 28530: (QA follow-up) Fix N+1 query performance issue > >Refactored lowest_ratio_library() to eliminate the N+1 query problem >by using aggregated queries with GROUP BY instead of per-branch queries. > >Performance improvement: >- Before: O(3n) queries where n = number of branches with float limits > Example: 20 branches = 60 database queries >- After: O(3) queries regardless of number of branches > Example: 20 branches = 3 database queries > >The optimization uses three aggregated queries: >1. Single query with GROUP BY to get item counts at all branches >2. Single query with GROUP BY to get in-transit-to counts for all branches >3. Single query with GROUP BY to get in-transit-from counts for all branches > >Results are combined in memory to calculate ratios, maintaining identical >logic and behavior while dramatically improving database efficiency. > >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >--- > Koha/Library/FloatLimits.pm | 215 +++++++++++++++++------------------- > 1 file changed, 100 insertions(+), 115 deletions(-) > >diff --git a/Koha/Library/FloatLimits.pm b/Koha/Library/FloatLimits.pm >index 2ed7efac98c..d22892fc51d 100644 >--- a/Koha/Library/FloatLimits.pm >+++ b/Koha/Library/FloatLimits.pm >@@ -97,123 +97,108 @@ sub lowest_ratio_library { > return unless @float_limits; > > my @candidates; >- my $use_item_level = C4::Context->preference('item-level_itypes'); >- >- foreach my $limit (@float_limits) { >- my $branch = $limit->get_column('branchcode'); >- my $float_limit_val = $limit->get_column('float_limit'); >- >- my $item_count; >- >- if ($use_item_level) { >- my $at_branch_count = Koha::Items->search( >- { >- itype => $item->effective_itemtype, >- holdingbranch => $branch, >- onloan => undef >- }, >- )->count; >- >- # Count items in transit TO this branch >- my $in_transit_to_count = Koha::Items->search( >- { >- itype => $item->effective_itemtype, >- >- # Join with active transfers where this branch is the destination >- }, >- { >- join => 'branchtransfers', >- where => { >- 'branchtransfers.tobranch' => $branch, >- 'branchtransfers.datearrived' => undef, # Still in transit >- 'branchtransfers.datecancelled' => undef, #Not cancelled >- }, >- distinct => 1 >- } >- )->count; >- >- my $in_transit_from_count = Koha::Items->search( >- { itype => $item->effective_itemtype }, >- { >- join => 'branchtransfers', >- where => { >- 'branchtransfers.frombranch' => $branch, >- 'branchtransfers.datearrived' => undef, # Still in transit >- 'branchtransfers.datecancelled' => undef, #Not cancelled >- }, >- distinct => 1 >- } >- )->count; >- $item_count = $at_branch_count + $in_transit_to_count - $in_transit_from_count; >- >- # artificially adjust counts for the item being checked in >- if ($from_branch) { >- >- # This is the checkin branch - artificially add 1 >- if ( $branch eq $branchcode ) { >- $item_count++; >- } >- >- # This is where the item came from - artificially subtract 1 >- if ( $branch eq $from_branch ) { >- $item_count--; >- } >+ my $use_item_level = C4::Context->preference('item-level_itypes'); >+ my $effective_itemtype = $item->effective_itemtype; >+ >+ # Build hash of float limits for quick lookup >+ my %float_limit_by_branch = map { $_->get_column('branchcode') => $_->get_column('float_limit') } @float_limits; >+ my @branch_codes = keys %float_limit_by_branch; >+ >+ # Get item counts at branches in a single aggregated query >+ my %at_branch_counts; >+ if ($use_item_level) { >+ my $at_branch_rs = $schema->resultset('Item')->search( >+ { >+ itype => $effective_itemtype, >+ holdingbranch => { -in => \@branch_codes }, >+ onloan => undef >+ }, >+ { >+ select => [ 'holdingbranch', { count => 'itemnumber', -as => 'item_count' } ], >+ as => [ 'holdingbranch', 'item_count' ], >+ group_by => ['holdingbranch'] > } >- >- } else { >- my $at_branch_count = Koha::Items->search( >- { >- holdingbranch => $branch, >- 'biblioitem.itemtype' => $item->effective_itemtype, >- onloan => undef >- }, >- )->count; >- >- # Count items in transit TO this branch >- my $in_transit_to_count = Koha::Items->search( >- { >- itype => $item->effective_itemtype, >- }, >- { >- join => 'branchtransfers', >- where => { >- 'branchtransfers.tobranch' => $branch, >- 'branchtransfers.datearrived' => undef, # Still in transit >- 'branchtransfers.datecancelled' => undef, #Not cancelled >- }, >- distinct => 1 >- } >- )->count; >- >- my $in_transit_from_count = Koha::Items->search( >- { >- itype => $item->effective_itemtype, >- }, >- { >- join => 'branchtransfers', >- where => { >- 'branchtransfers.frombranch' => $branch, >- 'branchtransfers.datearrived' => undef, # Still in transit >- 'branchtransfers.datecancelled' => undef, #Not cancelled >- }, >- distinct => 1 >- } >- )->count; >- $item_count = $at_branch_count + $in_transit_to_count - $in_transit_from_count; >- >- # artificially adjust counts for the item being checked in >- if ($from_branch) { >- >- # This is the checkin branch - artificially add 1 >- if ( $branch eq $branchcode ) { >- $item_count++; >- } >- >- # This is where the item came from - artificially subtract 1 >- if ( $branch eq $from_branch ) { >- $item_count--; >- } >+ ); >+ while ( my $row = $at_branch_rs->next ) { >+ $at_branch_counts{ $row->get_column('holdingbranch') } = $row->get_column('item_count'); >+ } >+ } else { >+ my $at_branch_rs = $schema->resultset('Item')->search( >+ { >+ holdingbranch => { -in => \@branch_codes }, >+ 'biblioitem.itemtype' => $effective_itemtype, >+ onloan => undef >+ }, >+ { >+ join => 'biblioitem', >+ select => [ 'me.holdingbranch', { count => 'me.itemnumber', -as => 'item_count' } ], >+ as => [ 'holdingbranch', 'item_count' ], >+ group_by => ['me.holdingbranch'] > } >+ ); >+ while ( my $row = $at_branch_rs->next ) { >+ $at_branch_counts{ $row->get_column('holdingbranch') } = $row->get_column('item_count'); >+ } >+ } >+ >+ # Get items in transit TO branches in a single aggregated query >+ my %in_transit_to_counts; >+ my $to_transit_rs = $schema->resultset('Item')->search( >+ { 'me.itype' => $effective_itemtype }, >+ { >+ join => 'branchtransfers', >+ where => { >+ 'branchtransfers.tobranch' => { -in => \@branch_codes }, >+ 'branchtransfers.datearrived' => undef, >+ 'branchtransfers.datecancelled' => undef, >+ }, >+ select => [ 'branchtransfers.tobranch', { count => { distinct => 'me.itemnumber' }, -as => 'item_count' } ], >+ as => [ 'tobranch', 'item_count' ], >+ group_by => ['branchtransfers.tobranch'] >+ } >+ ); >+ while ( my $row = $to_transit_rs->next ) { >+ $in_transit_to_counts{ $row->get_column('tobranch') } = $row->get_column('item_count'); >+ } >+ >+ # Get items in transit FROM branches in a single aggregated query >+ my %in_transit_from_counts; >+ my $from_transit_rs = $schema->resultset('Item')->search( >+ { 'me.itype' => $effective_itemtype }, >+ { >+ join => 'branchtransfers', >+ where => { >+ 'branchtransfers.frombranch' => { -in => \@branch_codes }, >+ 'branchtransfers.datearrived' => undef, >+ 'branchtransfers.datecancelled' => undef, >+ }, >+ select => >+ [ 'branchtransfers.frombranch', { count => { distinct => 'me.itemnumber' }, -as => 'item_count' } ], >+ as => [ 'frombranch', 'item_count' ], >+ group_by => ['branchtransfers.frombranch'] >+ } >+ ); >+ while ( my $row = $from_transit_rs->next ) { >+ $in_transit_from_counts{ $row->get_column('frombranch') } = $row->get_column('item_count'); >+ } >+ >+ # Calculate ratios for each branch using the aggregated counts >+ foreach my $branch (@branch_codes) { >+ my $float_limit_val = $float_limit_by_branch{$branch}; >+ >+ my $item_count = >+ ( $at_branch_counts{$branch} || 0 ) + >+ ( $in_transit_to_counts{$branch} || 0 ) - >+ ( $in_transit_from_counts{$branch} || 0 ); >+ >+ # Artificially adjust counts for the item being checked in >+ if ($from_branch) { >+ >+ # This is the checkin branch - artificially add 1 >+ $item_count++ if $branch eq $branchcode; >+ >+ # This is where the item came from - artificially subtract 1 >+ $item_count-- if $branch eq $from_branch; > } > > # Guard against division by zero (float_limit_val should never be 0 due to search filter, but be safe) >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28530
:
160038
|
160039
|
160040
|
160041
|
160042
|
160043
|
160044
|
160045
|
160046
|
160047
|
160048
|
160049
|
160050
|
160053
|
160054
|
160055
|
160056
|
160057
|
160058
|
160061
|
160062
|
160063
|
160064
|
160065
|
160066
|
160067
|
160068
|
160071
|
160072
|
160073
|
160074
|
160075
|
160076
|
160077
|
160078
|
160079
|
160080
|
160081
|
160082
|
160083
|
160084
|
163919
|
168343
|
168344
|
168345
|
168346
|
168347
|
168348
|
168349
|
168350
|
178762
|
178763
|
178764
|
178765
|
178766
|
178767
|
178768
|
178769
|
178783
|
178784
|
178785
|
178934
|
178935
|
178936
|
178937
|
178938
|
178939
|
178940
|
178941
|
178942
|
178943
|
178944
|
178945
|
178946
|
179943
|
179944
|
179945
|
179946
|
179947
|
179948
|
179949
|
179950
|
179951
|
179952
|
179953
|
179954
|
179955
|
180028
|
182931
|
184754
|
184755
|
184917
|
184936
|
184951
|
184952
|
184953
|
184954
|
184955
|
184956
|
184957
|
184958
|
184959
|
184960
|
184961
|
184963
|
184965
|
184966
|
184967
|
185038
|
185836
|
185837
|
185838
|
185839
|
185840
|
185841
|
185842
|
185843
|
187525
|
187601
|
190501
|
190502
|
190503
|
190504
|
190505
|
190506
|
190507
|
190508
|
190509
|
190510
|
190511
|
190512
|
190513
|
190547
|
190594
|
190595
|
190596
|
190597
|
190598
|
190599
|
190600
|
190601
|
190602
|
190603
|
190604
|
190605
|
190606
|
190607
|
192476
|
192477
|
192478
|
192479
|
192480
|
192481
|
192482
|
192483
|
192484
|
192485
|
192486
|
192487
|
192488
|
192489
|
192490
|
192491
|
192492
|
192493
| 192494 |
192495
|
192918
|
192954