From bb13e462691c044ab9d79347be1e3feccd1e8bbe Mon Sep 17 00:00:00 2001
From: Robin Sheat <robin@catalyst.net.nz>
Date: Tue, 15 Oct 2013 15:01:31 +1300
Subject: [PATCH] Bug 11051 - remove unneccessary SQL queries in GetBranches

The way GetBranches was written, it will issue one query to get all
branches, and then one query per branch for the branch relations.
This patch pre-fetches the relations table (as we need it all anyway)
and so makes the whole process happen in two queries, rather than take
1+N, where N is the number of branches.

This might not seem like much, but when you do a search, GetBranches is
called once for each result, so 25. And you might have 10 branches. This
causes 275 database requests when you could get away with 50.

From profiling, when you run a search, this is the thing that calls
DBI::st::execute the most. Refer:
http://debian.koha-community.org/~robin/opac-search/usr-share-koha-lib-C4-Branch-pm-146-line.html#125

Test Plan:
* Have a database with branches and relationships between the branches.
  (these are 'Lubrary groups' in the UI.
* Make sure the relationships show up correctly after applying the
  patch.
---
 C4/Branch.pm | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/C4/Branch.pm b/C4/Branch.pm
index bdc01dc..9194c8c 100644
--- a/C4/Branch.pm
+++ b/C4/Branch.pm
@@ -70,8 +70,7 @@ The functions in this module deal with branches.
   $branches = &GetBranches();
 
 Returns informations about ALL branches, IndependentBranches Insensitive.
-GetBranchInfo() returns the same information without the problems of this function 
-(namespace collision, mainly).
+GetBranchInfo() returns the same information.
 
 Create a branch selector with the following code.
 
@@ -106,38 +105,32 @@ Create a branch selector with the following code.
 =cut
 
 sub GetBranches {
-    my ($onlymine)=@_;
+    my ($onlymine) = @_;
+
     # returns a reference to a hash of references to ALL branches...
     my %branches;
     my $dbh = C4::Context->dbh;
     my $sth;
-    my $query="SELECT * FROM branches";
+    my $query = "SELECT * FROM branches";
     my @bind_parameters;
-    if ($onlymine && C4::Context->userenv && C4::Context->userenv->{branch}){
-      $query .= ' WHERE branchcode = ? ';
-      push @bind_parameters, C4::Context->userenv->{branch};
+    if ( $onlymine && C4::Context->userenv && C4::Context->userenv->{branch} ) {
+        $query .= ' WHERE branchcode = ? ';
+        push @bind_parameters, C4::Context->userenv->{branch};
     }
-        $query.=" ORDER BY branchname";
+    $query .= " ORDER BY branchname";
     $sth = $dbh->prepare($query);
-    $sth->execute( @bind_parameters );
-
-    my $nsth = $dbh->prepare(
-        "SELECT categorycode FROM branchrelations WHERE branchcode = ?"
-    );  # prepare once, outside while loop
+    $sth->execute(@bind_parameters);
+
+    my $relations_sth =
+      $dbh->prepare("SELECT branchcode,categorycode FROM branchrelations");
+    $relations_sth->execute();
+    my %relations;
+    while ( my $rel = $relations_sth->fetchrow_hashref ) {
+        push @{ $relations{ $rel->{branchcode} } }, $rel->{categorycode};
+    }
 
     while ( my $branch = $sth->fetchrow_hashref ) {
-        $nsth->execute( $branch->{'branchcode'} );
-        while ( my ($cat) = $nsth->fetchrow_array ) {
-            # FIXME - This seems wrong. It ought to be
-            # $branch->{categorycodes}{$cat} = 1;
-            # otherwise, there's a namespace collision if there's a
-            # category with the same name as a field in the 'branches'
-            # table (i.e., don't create a category called "issuing").
-            # In addition, the current structure doesn't really allow
-            # you to list the categories that a branch belongs to:
-            # you'd have to list keys %$branch, and remove those keys
-            # that aren't fields in the "branches" table.
-         #   $branch->{$cat} = 1;
+        foreach my $cat ( @{ $relations{ $branch->{branchcode} } } ) {
             $branch->{category}{$cat} = 1;
         }
         $branches{ $branch->{'branchcode'} } = $branch;
@@ -146,7 +139,7 @@ sub GetBranches {
 }
 
 sub onlymine {
-    return 
+    return
     C4::Context->preference('IndependentBranches') &&
     C4::Context->userenv                           &&
     C4::Context->userenv->{flags} %2 != 1          &&
-- 
1.8.1.2