Bugzilla – Attachment 2692 Details for
Bug 5304
Too many post find queries for items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Proposed patch
[PATCH] bug 5304: GetItemsInfo() - optimise query.mbox (text/plain), 15.67 KB, created by
Robin Sheat
on 2010-10-18 21:29:27 UTC
(
hide
)
Description:
Proposed patch
Filename:
MIME Type:
Creator:
Robin Sheat
Created:
2010-10-18 21:29:27 UTC
Size:
15.67 KB
patch
obsolete
>From srdjan@catalyst.net.nz Mon Oct 18 15:23:44 2010 >Return-Path: <srdjan@stuffdev.wgtn.cat-it.co.nz> >X-Original-To: robin@catalyst.net.nz >Delivered-To: robin@catalyst.net.nz >Received: from localhost (localhost [127.0.0.1]) by mail.catalyst.net.nz > (Postfix) with ESMTP id 354BE31E18; Mon, 18 Oct 2010 15:23:44 +1300 (NZDT) >X-Virus-Scanned: Debian amavisd-new at catalyst.net.nz >Received: from mail.catalyst.net.nz ([127.0.0.1]) by localhost > (bertrand.catalyst.net.nz [127.0.0.1]) (amavisd-new, port 10024) with ESMTP > id n6+eyr+h-UWc; Mon, 18 Oct 2010 15:23:41 +1300 (NZDT) >Received: from stuffdev.wgtn.cat-it.co.nz (unknown > [IPv6:2404:130:0:1000:205:1cff:fe1d:c4e2]) by mail.catalyst.net.nz > (Postfix) with ESMTPS id D8D7A31C73; Mon, 18 Oct 2010 15:23:41 +1300 (NZDT) >Received: from srdjan by stuffdev.wgtn.cat-it.co.nz with local (Exim 4.69) > (envelope-from <srdjan@stuffdev.wgtn.cat-it.co.nz>) id 1P7fP5-0001KR-3M; > Mon, 18 Oct 2010 15:24:47 +1300 >From: Srdjan Jankovic <srdjan@catalyst.net.nz> >To: robin@catalyst.net.nz >Cc: Srdjan Jankovic <srdjan@catalyst.net.nz> >Subject: [PATCH] bug 5304: GetItemsInfo() - optimise query >Date: Mon, 18 Oct 2010 15:24:39 +1300 >Message-Id: <1287368679-5076-1-git-send-email-srdjan@catalyst.net.nz> >X-Mailer: git-send-email 1.6.5 >X-Evolution-Source: imap://robin@mail.catalyst.net.nz/ >Content-Transfer-Encoding: 8bit >Mime-Version: 1.0 > >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. >--- > 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.6.5 > >
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 5304
:
2692
|
29596
|
29597
|
29606
|
29607
|
29629
|
29630
|
29631
|
32722
|
32723