Attachment 75258 Details for Bug 19792 – Bug 19792: Performance boost for GetBudgetHierarchy

[patch] Bug 19792: Performance boost for GetBudgetHierarchy

Bug-19792-Performance-boost-for-GetBudgetHierarchy.patch (text/plain), 5.27 KB, created by Marcel de Rooy on 2018-05-11 07:58:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2018-05-11 07:58:00 UTC

Size: 5.27 KB

patch

obsolete

>From 0c114a5b7cafe3996ccce8d884a9fb38fb10b6e1 Mon Sep 17 00:00:00 2001
>From: David Bourgault <david.bourgault@inlibro.com>
>Date: Mon, 11 Dec 2017 11:56:27 -0500
>Subject: [PATCH] Bug 19792: Performance boost for GetBudgetHierarchy
>Content-Type: text/plain; charset=utf-8
>
>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 <J.P.Knight@lboro.ac.uk>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
>---
> C4/Budgets.pm       | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> acqui/acqui-home.pl |  3 ---
> 2 files changed, 60 insertions(+), 9 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 d23f082..9c70773 100755
>--- a/acqui/acqui-home.pl
>+++ b/acqui/acqui-home.pl
>@@ -78,9 +78,6 @@ foreach my $budget ( @{$budget_arr} ) {
>     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

Actions: View | Diff | Splinter Review

Attachments on bug 19792: 69702 | 69960 | 70296 | 70297 | 70298 | 70306 | 71989 | 71990 | 71991 | 74983 | 74984 | 74985 | 75258 | 75259 | 75260