I just spotted a glitch with this in testing. Please don't use this patch, a new one will follow
Chris * Robin Sheat ([email protected]) wrote: > From: Srdjan Jankovic <[email protected]> > > 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 <[email protected]> > --- > 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 > > _______________________________________________ > Koha-patches mailing list > [email protected] > http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-patches -- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
signature.asc
Description: Digital signature
_______________________________________________ Koha-patches mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-patches
