[Koha-patches] [PATCH] bug 5304: GetItemsInfo() - optimise query
Robin Sheat
robin at catalyst.net.nz
Mon Oct 18 23:28:44 CEST 2010
From: Srdjan Jankovic <srdjan at catalyst.net.nz>
GetItemsInfo(): combine queries so we get as mch as possible in one go.
CheckReserves() - extract MostImportantReserve((), so we don't need to
execute items query if we already have details
Create index (biblionumber, biblioitemnumber) on reserveconstraints.
Signed-off-by: Robin Sheat <robin at catalyst.net.nz>
---
C4/Items.pm | 210 +++++++++++++++-----------------
C4/Reserves.pm | 55 ++++++---
installer/data/mysql/kohastructure.sql | 3 +-
installer/data/mysql/updatedatabase.pl | 7 +
4 files changed, 145 insertions(+), 130 deletions(-)
diff --git a/C4/Items.pm b/C4/Items.pm
index 794bff7..5366222 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -2,6 +2,8 @@ package C4::Items;
# Copyright 2007 LibLime, Inc.
#
+# Parts copyright Catalyst IT 2010
+#
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it under the
@@ -1168,130 +1170,119 @@ If this is set, it is set to C<One Order>.
sub GetItemsInfo {
my ( $biblionumber, $type ) = @_;
my $dbh = C4::Context->dbh;
+
+ # get notforloan complete status if applicable
+ my $sthnflstatus = $dbh->prepare(
+ "SELECT authorised_value
+ FROM marc_subfield_structure
+ WHERE kohafield='items.notforloan'
+ "
+ );
+ $sthnflstatus->execute;
+ my ($authorised_value_nfl) = $sthnflstatus->fetchrow;
+
+ # my stack procedures
+ my $stackstatus = $dbh->prepare(
+ 'SELECT authorised_value
+ FROM marc_subfield_structure
+ WHERE kohafield="items.stack"
+ '
+ );
+ $stackstatus->execute;
+ my ($authorised_value_stack) = $stackstatus->fetchrow;
+
+ my $user_branch;
+ if (C4::Context->preference("IndependantBranches")){
+ my $userenv = C4::Context->userenv;
+ $user_branch = $userenv->{branch} if $userenv && ( $userenv->{flags} % 2 != 1 );
+ }
+
# note biblioitems.* must be avoided to prevent large marc and marcxml fields from killing performance.
- my $query = "
- SELECT items.*,
- biblio.*,
- biblioitems.volume,
- biblioitems.number,
- biblioitems.itemtype,
- biblioitems.isbn,
- biblioitems.issn,
- biblioitems.publicationyear,
- biblioitems.publishercode,
- biblioitems.volumedate,
- biblioitems.volumedesc,
- biblioitems.lccn,
- biblioitems.url,
- items.notforloan as itemnotforloan,
- itemtypes.description,
- itemtypes.notforloan as notforloan_per_itemtype,
- branchurl
- FROM items
- LEFT JOIN branches ON items.homebranch = branches.branchcode
- LEFT JOIN biblio ON biblio.biblionumber = items.biblionumber
- LEFT JOIN biblioitems ON biblioitems.biblioitemnumber = items.biblioitemnumber
- LEFT JOIN itemtypes ON itemtypes.itemtype = "
- . (C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype');
- $query .= " WHERE items.biblionumber = ? ORDER BY branches.branchname,items.dateaccessioned desc" ;
+ my $fields = <<EOS;
+ items.*,
+ biblio.*,
+ issues.*,
+ biblioitems.volume,
+ biblioitems.number,
+ biblioitems.itemtype,
+ biblioitems.isbn,
+ biblioitems.issn,
+ biblioitems.publicationyear,
+ biblioitems.publishercode,
+ biblioitems.volumedate,
+ biblioitems.volumedesc,
+ biblioitems.lccn,
+ biblioitems.url,
+ items.notforloan as itemnotforloan,
+ itemtypes.description,
+ itemtypes.notforloan as notforloan_per_itemtype,
+ homebranch.branchurl,
+ COALESCE(holdingbranch.branchname, homebranch.branchname) AS branchname,
+ serial.serialseq,
+ serial.publisheddate
+EOS
+ my $itemtypes_join_field = C4::Context->preference('item-level_itypes') ? 'items.itype'
+ : 'biblioitems.itemtype';
+ my $from = <<EOS;
+items
+LEFT JOIN biblio USING(biblionumber)
+LEFT JOIN biblioitems USING(biblioitemnumber)
+LEFT OUTER JOIN (SELECT itemnumber, serialseq, publisheddate
+ FROM serialitems LEFT JOIN serial USING(serialid)) serial
+ USING(itemnumber)
+LEFT OUTER JOIN (SELECT itemnumber, borrowernumber, cardnumber, surname, firstname, date_due,
+ borrowers.branchcode AS borrower_branchcode
+ FROM issues LEFT JOIN borrowers USING(borrowernumber)) issues
+ USING(itemnumber)
+LEFT JOIN itemtypes ON itemtypes.itemtype = $itemtypes_join_field
+LEFT JOIN branches homebranch ON items.homebranch = homebranch.branchcode
+LEFT OUTER JOIN branches holdingbranch ON items.holdingbranch = holdingbranch.branchcode
+EOS
+ if ($authorised_value_nfl) {
+ $from .= <<EOS;
+LEFT OUTER JOIN authorised_values authorised_values_nfl
+ ON authorised_values_nfl.category = '$authorised_value_nfl'
+ AND authorised_values_nfl.authorised_value = items.notforloan
+EOS
+ $fields .= ", authorised_values_nfl.lib AS notforloanvalue";
+ }
+
+ if ($authorised_value_stack) {
+ $from .= <<EOS;
+LEFT OUTER JOIN authorised_values authorised_values_stack
+ ON authorised_values_nfl.category = '$authorised_value_stack'
+ AND authorised_values_nfl.authorised_value = items.stack
+EOS
+ $fields .= ", authorised_values_stack.lib AS stack";
+ }
+
+ my $query = "SELECT $fields FROM $from WHERE items.biblionumber = ?
+ ORDER BY homebranch.branchname,items.dateaccessioned desc" ;
my $sth = $dbh->prepare($query);
$sth->execute($biblionumber);
- my $i = 0;
+
my @results;
my $serial;
-
- my $isth = $dbh->prepare(
- "SELECT issues.*,borrowers.cardnumber,borrowers.surname,borrowers.firstname,borrowers.branchcode as bcode
- FROM issues LEFT JOIN borrowers ON issues.borrowernumber=borrowers.borrowernumber
- WHERE itemnumber = ?"
- );
- my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=? ");
while ( my $data = $sth->fetchrow_hashref ) {
- my $datedue = '';
+ my $datedue = $data->{'date_due'};
my $count_reserves;
- $isth->execute( $data->{'itemnumber'} );
- if ( my $idata = $isth->fetchrow_hashref ) {
- $data->{borrowernumber} = $idata->{borrowernumber};
- $data->{cardnumber} = $idata->{cardnumber};
- $data->{surname} = $idata->{surname};
- $data->{firstname} = $idata->{firstname};
- $datedue = $idata->{'date_due'};
- if (C4::Context->preference("IndependantBranches")){
- my $userenv = C4::Context->userenv;
- if ( ($userenv) && ( $userenv->{flags} % 2 != 1 ) ) {
- $data->{'NOTSAMEBRANCH'} = 1 if ($idata->{'bcode'} ne $userenv->{branch});
- }
- }
- }
- if ( $data->{'serial'}) {
- $ssth->execute($data->{'itemnumber'}) ;
- ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array();
- $serial = 1;
- }
- if ( $datedue eq '' ) {
+
+ $data->{'NOTSAMEBRANCH'} = $data->{'borrower_branchcode'} ne $user_branch
+ if $data->{'borrower_branchcode'} && $user_branch;
+
+ $serial = 1 if $data->{'serial'};
+
+ unless ( $datedue ) {
my ( $restype, $reserves ) =
- C4::Reserves::CheckReserves( $data->{'itemnumber'} );
+ C4::Reserves::MostImportantReserve( $data->{'biblioitemnumber'}, $data->{'biblionumber'}, $data->{'itemnumber'} );
# Previous conditional check with if ($restype) is not needed because a true
# result for one item will result in subsequent items defaulting to this true
# value.
$count_reserves = $restype;
}
- #get branch information.....
- my $bsth = $dbh->prepare(
- "SELECT * FROM branches WHERE branchcode = ?
- "
- );
- $bsth->execute( $data->{'holdingbranch'} );
- if ( my $bdata = $bsth->fetchrow_hashref ) {
- $data->{'branchname'} = $bdata->{'branchname'};
- }
$data->{'datedue'} = $datedue;
$data->{'count_reserves'} = $count_reserves;
- # get notforloan complete status if applicable
- my $sthnflstatus = $dbh->prepare(
- 'SELECT authorised_value
- FROM marc_subfield_structure
- WHERE kohafield="items.notforloan"
- '
- );
-
- $sthnflstatus->execute;
- my ($authorised_valuecode) = $sthnflstatus->fetchrow;
- if ($authorised_valuecode) {
- $sthnflstatus = $dbh->prepare(
- "SELECT lib FROM authorised_values
- WHERE category=?
- AND authorised_value=?"
- );
- $sthnflstatus->execute( $authorised_valuecode,
- $data->{itemnotforloan} );
- my ($lib) = $sthnflstatus->fetchrow;
- $data->{notforloanvalue} = $lib;
- }
-
- # my stack procedures
- my $stackstatus = $dbh->prepare(
- 'SELECT authorised_value
- FROM marc_subfield_structure
- WHERE kohafield="items.stack"
- '
- );
- $stackstatus->execute;
-
- ($authorised_valuecode) = $stackstatus->fetchrow;
- if ($authorised_valuecode) {
- $stackstatus = $dbh->prepare(
- "SELECT lib
- FROM authorised_values
- WHERE category=?
- AND authorised_value=?
- "
- );
- $stackstatus->execute( $authorised_valuecode, $data->{stack} );
- my ($lib) = $stackstatus->fetchrow;
- $data->{stack} = $lib;
- }
# Find the last 3 people who borrowed this item.
my $sth2 = $dbh->prepare("SELECT * FROM old_issues,borrowers
WHERE itemnumber = ?
@@ -1307,8 +1298,7 @@ sub GetItemsInfo {
$ii++;
}
- $results[$i] = $data;
- $i++;
+ push @results, $data;
}
if($serial) {
return( sort { ($b->{'publisheddate'} || $b->{'enumchron'}) cmp ($a->{'publisheddate'} || $a->{'enumchron'}) } @results );
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 9b7014e..dc514e1 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -797,26 +797,10 @@ sub GetReserveStatus {
($status, $reserve) = &CheckReserves($itemnumber);
($status, $reserve) = &CheckReserves(undef, $barcode);
-Find a book in the reserves.
+Find the book by its itemnumber or barcode, and call C<MostImportantReserve>
C<$itemnumber> is the book's item number.
-
-As I understand it, C<&CheckReserves> looks for the given item in the
-reserves. If it is found, that's a match, and C<$status> is set to
-C<Waiting>.
-
-Otherwise, it finds the most important item in the reserves with the
-same biblio number as this book (I'm not clear on this) and returns it
-with C<$status> set to C<Reserved>.
-
-C<&CheckReserves> returns a two-element list:
-
-C<$status> is either C<Waiting>, C<Reserved> (see above), or 0.
-
-C<$reserve> is the reserve item that matched. It is a
-reference-to-hash whose keys are mostly the fields of the reserves
-table in the Koha database.
-
+C<$barcode> is the book's barcode
=cut
sub CheckReserves {
@@ -851,6 +835,39 @@ sub CheckReserves {
# execpt where items.notforloan < 0 : This indicates the item is holdable.
return ( 0, 0 ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
+ return MostImportantReserve($bibitem, $biblio, $itemnumber);
+}
+
+=head2 MostImportantReserve
+
+ ($status, $reserve) = &MostImportantReserve($itemnumber);
+
+Find the most important reserve for a book.
+
+C<$bibitem> is the book's biblioitem number.
+C<$biblio> is the book's biblio number.
+C<$itemnumber> is the book's item number.
+
+As I understand it, C<&MostImportantReserve> looks for the given item in the
+reserves. If it is found, that's a match, and C<$status> is set to
+C<Waiting>.
+
+Otherwise, it finds the most important item in the reserves with the
+same biblio number as this book (I'm not clear on this) and returns it
+with C<$status> set to C<Reserved>.
+
+C<&MostImportantReserve> returns a two-element list:
+
+C<$status> is either C<Waiting>, C<Reserved> (see above), or 0.
+
+C<$reserve> is the reserve item that matched. It is a
+reference-to-hash whose keys are mostly the fields of the reserves
+table in the Koha database.
+
+=cut
+
+sub MostImportantReserve {
+ my ( $bibitem, $biblio, $itemnumber ) = @_;
# Find this item in the reserves
my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber );
@@ -877,7 +894,7 @@ sub CheckReserves {
# If we get this far, then no exact match was found.
# We return the most important (i.e. next) reservation.
if ($highest) {
- $highest->{'itemnumber'} = $item;
+ $highest->{'itemnumber'} = $itemnumber;
return ( "Reserved", $highest );
}
else {
diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index e42652c..b04a9c5 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -1517,7 +1517,8 @@ CREATE TABLE `reserveconstraints` (
`reservedate` date default NULL,
`biblionumber` int(11) NOT NULL default 0,
`biblioitemnumber` int(11) default NULL,
- `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP
+ `timestamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP,
+ KEY `biblio` (`biblionumber`, `biblioitemnumber`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
--
diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index 2bd4e21..3fb576f 100755
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -3744,6 +3744,13 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
SetVersion ($DBversion);
}
+$DBversion = "3.01.00.XXX";
+if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
+ $dbh->do("CREATE INDEX `reserveconstraints_biblio_idx` ON reserveconstraints (`biblionumber`, `biblioitemnumber`) ");
+ print "Upgrade to $DBversion done (reserveconstraints_biblio_idx index added)\n";
+ SetVersion ($DBversion);
+}
+
=item DropAllForeignKeys($table)
--
1.7.1
More information about the Koha-patches
mailing list