[Koha-patches] [PATCH] bug 5304: GetItemsInfo() - optimise query
Chris Cormack
chrisc at catalyst.net.nz
Mon Oct 18 23:52:43 CEST 2010
I just spotted a glitch with this in testing.
Please don't use this patch, a new one will follow
Chris
* Robin Sheat (robin at catalyst.net.nz) wrote:
> 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
>
> _______________________________________________
> Koha-patches mailing list
> Koha-patches at lists.koha-community.org
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: </pipermail/koha-patches/attachments/20101019/c0398ebc/attachment.pgp>
More information about the Koha-patches
mailing list