From d7f3356f5264b3e13afe8a44b32183df909e9a03 Mon Sep 17 00:00:00 2001 From: David Bourgault Date: Mon, 11 Dec 2017 11:56:27 -0500 Subject: [PATCH] Bug 19792 - Perf. boost for GetBudgetHierarchy This patches reduces the number of SQL calls by combining multiple calls to the recursive functions GetBudgetSpent(), GetBudgetOrdered(), etc. into 4 big queries. It also removes duplicate function calls from acqui-home.pl Test plan: 0) Visit Acquisition home 0) Apply patch 1) Refresh page. It shoud look identical. 2) prove t/db_dependent/budgets.t Signed-off-by: Jon Knight --- C4/Budgets.pm | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----- acqui/acqui-home.pl | 11 --------- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/C4/Budgets.pm b/C4/Budgets.pm index 10f5147..6d2718a 100644 --- a/C4/Budgets.pm +++ b/C4/Budgets.pm @@ -480,10 +480,12 @@ sub GetBudgetHierarchy { my @bind_params; my $dbh = C4::Context->dbh; my $query = qq| - SELECT aqbudgets.*, aqbudgetperiods.budget_period_active, aqbudgetperiods.budget_period_description + SELECT aqbudgets.*, aqbudgetperiods.budget_period_active, aqbudgetperiods.budget_period_description, + b.firstname as budget_owner_firstname, b.surname as budget_owner_surname, b.borrowernumber as budget_owner_borrowernumber FROM aqbudgets + LEFT JOIN borrowers b on b.borrowernumber = aqbudgets.budget_owner_id JOIN aqbudgetperiods USING (budget_period_id)|; - + my @where_strings; # show only period X if requested if ($budget_period_id) { @@ -541,12 +543,64 @@ sub GetBudgetHierarchy { _add_budget_children(\@sort, $first_parent, 0); } + # Get all the budgets totals in as few queries as possible + my $hr_budget_spent = $dbh->selectall_hashref(qq| + SELECT aqorders.budget_id, aqbudgets.budget_parent_id, + SUM( COALESCE(unitprice_tax_included, ecost_tax_included) * quantity ) AS budget_spent + FROM aqorders JOIN aqbudgets USING (budget_id) + WHERE quantityreceived > 0 AND datecancellationprinted IS NULL + GROUP BY budget_id + |, 'budget_id'); + my $hr_budget_ordered = $dbh->selectall_hashref(qq| + SELECT aqorders.budget_id, aqbudgets.budget_parent_id, + SUM(ecost_tax_included * quantity) AS budget_ordered + FROM aqorders JOIN aqbudgets USING (budget_id) + WHERE quantityreceived = 0 AND datecancellationprinted IS NULL + GROUP BY budget_id + |, 'budget_id'); + my $hr_budget_spent_shipment = $dbh->selectall_hashref(qq| + SELECT shipmentcost_budgetid as budget_id, + SUM(shipmentcost) as shipmentcost + FROM aqinvoices + WHERE closedate IS NOT NULL + GROUP BY shipmentcost_budgetid + |, 'budget_id'); + my $hr_budget_ordered_shipment = $dbh->selectall_hashref(qq| + SELECT shipmentcost_budgetid as budget_id, + SUM(shipmentcost) as shipmentcost + FROM aqinvoices + WHERE closedate IS NULL + GROUP BY shipmentcost_budgetid + |, 'budget_id'); + + my $recursiveAdd; + $recursiveAdd = sub { + my ($budget, $parent) = @_; + + foreach my $child (@{$budget->{children}}){ + $recursiveAdd->($child, $budget); + } + + $budget->{budget_spent} += $hr_budget_spent->{$budget->{budget_id}}->{budget_spent}; + $budget->{budget_spent} += $hr_budget_spent_shipment->{$budget->{budget_id}}->{shipmentcost}; + $budget->{budget_ordered} += $hr_budget_ordered->{$budget->{budget_id}}->{budget_ordered}; + $budget->{budget_ordered} += $hr_budget_ordered_shipment->{$budget->{budget_id}}->{shipmentcost}; + + $budget->{total_spent} += $budget->{budget_spent}; + $budget->{total_ordered} += $budget->{budget_ordered}; + + if ($parent) { + $parent->{total_spent} += $budget->{total_spent}; + $parent->{total_ordered} += $budget->{total_ordered}; + } + }; + foreach my $budget (@sort) { - $budget->{budget_spent} = GetBudgetSpent( $budget->{budget_id} ); - $budget->{budget_ordered} = GetBudgetOrdered( $budget->{budget_id} ); - $budget->{total_spent} = GetBudgetHierarchySpent( $budget->{budget_id} ); - $budget->{total_ordered} = GetBudgetHierarchyOrdered( $budget->{budget_id} ); + if ($budget->{budget_parent_id} == undef) { + $recursiveAdd->($budget); + } } + return \@sort; } diff --git a/acqui/acqui-home.pl b/acqui/acqui-home.pl index 4da1eaf..f3b57ec 100755 --- a/acqui/acqui-home.pl +++ b/acqui/acqui-home.pl @@ -71,20 +71,9 @@ my @budget_loop; foreach my $budget ( @{$budget_arr} ) { next unless (CanUserUseBudget($loggedinuser, $budget, $userflags)); - my $patron = Koha::Patrons->find( $budget->{budget_owner_id} ); - if ( $patron ) { - # FIXME should pass the entire object into budget_owner - $budget->{budget_owner_firstname} = $patron->firstname; - $budget->{budget_owner_surname} = $patron->surname; - $budget->{budget_owner_borrowernumber} = $patron->borrowernumber; - } - if ( !defined $budget->{budget_amount} ) { $budget->{budget_amount} = 0; } - - $budget->{'budget_ordered'} = GetBudgetOrdered( $budget->{'budget_id'} ); - $budget->{'budget_spent'} = GetBudgetSpent( $budget->{'budget_id'} ); if ( !defined $budget->{budget_spent} ) { $budget->{budget_spent} = 0; } -- 2.1.4