From 44a56d7e87ca32ce046a0df59d6acec19187dc14 Mon Sep 17 00:00:00 2001 From: Ian Walls Date: Wed, 31 Aug 2011 14:43:09 -0400 Subject: [PATCH] Bug 6801: checkoverdues returns unnecessary fields, causing slowness Content-Type: text/plain; charset="utf-8" Explicitly specifies which fields to return in C4::Overdues::checkoverdues SQL: all of biblio, items, and issues, and everything in biblioitems EXCEPT marc, marcxml and timestamp. Bug 6801: member details page taking long time to load when many checkouts present This patch removes the call to GetMemberDetails in build_issue_data; this heavy-weight subroutine was being run for every single item a patron (or their relatives) have checked out. Instead, the borrowers first name, surname and cardnumber are added to the GetPendingIssues query. I believe this is reasonable since GetPendingIssues can now return issues for multiple borrowers. Also corrects the $borrowernumber used for GetIssuesCharges and CanItemBeRenewed; was using the borrower whose page we were on, NOT the borrower of that specific item (which would be different in the Relatives Checkouts tab). Template calls to [% scope.borrowername %] are now broken up into [% scope.firstname %] [% scope.surname %]. Signed-off-by: Liz Rea On my test data, a patron with 180 checkouts (without this patch) would take more than a minute to bring back the circulation.pl and moremember.pl pages. With this patch, the time is reduced to 5 or so seconds. Big ups to Ian for tenaciously hunting this one down. --- C4/Members.pm | 6 +++- C4/Overdues.pm | 31 +++++++++++++++++++- circ/circulation.pl | 8 +---- .../prog/en/modules/circ/circulation.tt | 10 +++--- .../prog/en/modules/members/moremember.tt | 2 +- members/moremember.pl | 8 +---- 6 files changed, 45 insertions(+), 20 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 8d4a6c5..2795de1 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -1043,7 +1043,7 @@ sub GetPendingIssues { # Borrowers part of the query my $bquery = ''; for (my $i = 0; $i < @borrowernumbers; $i++) { - $bquery .= ' borrowernumber = ?'; + $bquery .= ' issues.borrowernumber = ?'; if ($i < $#borrowernumbers ) { $bquery .= ' OR'; } @@ -1070,6 +1070,9 @@ sub GetPendingIssues { biblioitems.volumedesc, biblioitems.lccn, biblioitems.url, + borrowers.firstname, + borrowers.surname, + borrowers.cardnumber, issues.timestamp AS timestamp, issues.renewals AS renewals, issues.borrowernumber AS borrowernumber, @@ -1078,6 +1081,7 @@ sub GetPendingIssues { LEFT JOIN items ON items.itemnumber = issues.itemnumber LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber + LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber WHERE $bquery ORDER BY issues.issuedate" diff --git a/C4/Overdues.pm b/C4/Overdues.pm index 28b135c..918926f 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -165,8 +165,37 @@ Returns a count and a list of overdueitems for a given borrowernumber sub checkoverdues { my $borrowernumber = shift or return; + # don't select biblioitems.marc or biblioitems.marcxml... too slow on large systems my $sth = C4::Context->dbh->prepare( - "SELECT * FROM issues + "SELECT biblio.*, items.*, issues.*, + biblioitems.volume, + bibliotiems.number, + biblioitems.itemtype, + biblioitems.isbn, + biblioitems.issn, + biblioitems.publicationyear, + biblioitems.publishercode, + biblioitems.volumedate, + biblioitems.volumedesc, + biblioitems.collectiontitle, + biblioitems.collectionissn, + biblioitems.collectionvolume, + biblioitems.editionstatement, + biblioitems.editionresponsibility, + biblioitems.illus, + biblioitems.pages, + biblioitems.notes, + biblioitems.size, + biblioitems.place, + biblioitems.lccn, + biblioitems.url, + biblioitems.cn_source, + biblioitems.cn_class, + biblioitems.cn_item, + biblioitems.cn_suffix, + biblioitems.cn_sort, + biblioitems.totalissues + FROM issues LEFT JOIN items ON issues.itemnumber = items.itemnumber LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber diff --git a/circ/circulation.pl b/circ/circulation.pl index fff32a2..8c28395 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -418,19 +418,15 @@ sub build_issue_data { foreach my $it ( @$issueslist ) { my $itemtypeinfo = getitemtypeinfo( (C4::Context->preference('item-level_itypes')) ? $it->{'itype'} : $it->{'itemtype'} ); - # Getting borrower details - my $memberdetails = GetMemberDetails($it->{'borrowernumber'}); - $it->{'borrowername'} = $memberdetails->{'firstname'} . " " . $memberdetails->{'surname'}; - $it->{'cardnumber'} = $memberdetails->{'cardnumber'}; # set itemtype per item-level_itype syspref - FIXME this is an ugly hack $it->{'itemtype'} = ( C4::Context->preference( 'item-level_itypes' ) ) ? $it->{'itype'} : $it->{'itemtype'}; ($it->{'charge'}, $it->{'itemtype_charge'}) = GetIssuingCharges( - $it->{'itemnumber'}, $borrower->{'borrowernumber'} + $it->{'itemnumber'}, $it->{'borrowernumber'} ); $it->{'charge'} = sprintf("%.2f", $it->{'charge'}); my ($can_renew, $can_renew_error) = CanBookBeRenewed( - $borrower->{'borrowernumber'},$it->{'itemnumber'} + $it->{'borrowernumber'},$it->{'itemnumber'} ); $it->{"renew_error_${can_renew_error}"} = 1 if defined $can_renew_error; my ( $restype, $reserves ) = CheckReserves( $it->{'itemnumber'} ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index d6b2b22..4f56504 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -701,7 +701,7 @@ No patron matched [% message %] [% todayissue.title |html %][% IF ( todayissue.author ) %], by [% todayissue.author %][% END %][% IF ( todayissue.itemnotes ) %]- [% todayissue.itemnotes %][% END %] [% todayissue.barcode %] [% UNLESS ( noItemTypeImages ) %] [% IF ( todayissue.itemtype_image ) %][% END %][% END %][% todayissue.itemtype %] [% todayissue.checkoutdate %] - [% IF ( todayissue.multiple_borrowers ) %][% todayissue.borrowername %][% END %] + [% IF ( todayissue.multiple_borrowers ) %][% todayissue.firstname %] [% todayissue.surname %][% END %] [% todayissue.issuingbranchname %] [% todayissue.itemcallnumber %] [% todayissue.charge %] @@ -776,7 +776,7 @@ No patron matched [% message %] [% previssue.itemtype %] [% previssue.displaydate %] - [% IF ( previssue.multiple_borrowers ) %][% previssue.borrowername %][% END %] + [% IF ( previssue.multiple_borrowers ) %][% previssue.firstname %] [% previssue.surname %][% END %] [% previssue.issuingbranchname %] [% previssue.itemcallnumber %] [% previssue.charge %] @@ -885,7 +885,7 @@ No patron matched [% message %] [% relissue.issuingbranchname %] [% relissue.itemcallnumber %] [% relissue.charge %] - [% relissue.replacementprice %][% relissue.borrowername %] ([% relissue.cardnumber %]) + [% relissue.replacementprice %][% relissue.firstname %] [% relissue.surname %] ([% relissue.cardnumber %]) [% END %] [% END %] @@ -905,10 +905,10 @@ No patron matched [% message %] [% relprevissue.displaydate %] [% relprevissue.issuingbranchname %] [% relprevissue.itemcallnumber %] - [% IF ( relprevissue.multiple_borrowers ) %][% relprevissue.borrowername %][% END %] + [% IF ( relprevissue.multiple_borrowers ) %][% relprevissue.firstname %] [% relprevissue.surname %][% END %] [% relprevissue.charge %] [% relprevissue.replacementprice %] - [% relprevissue.borrowername %] ([% relprevissue.cardnumber %]) + [% relprevissue.firstname %] [% relprevissue.surname %] ([% relprevissue.cardnumber %]) [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt index 84a9c48..3eb42f3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt @@ -566,7 +566,7 @@ function validate1(date) { [% relissueloo.itemcallnumber %] [% relissueloo.charge %] [% relissueloo.replacementprice %] - [% relissueloo.borrowername %] ([% relissueloo.cardnumber %]) + [% relissueloo.firstname %] [% relissueloo.surname %] ([% relissueloo.cardnumber %]) [% END %] diff --git a/members/moremember.pl b/members/moremember.pl index a76652f..b800199 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -259,10 +259,6 @@ sub build_issue_data { my $localissue; for ( my $i = 0 ; $i < $issuecount ; $i++ ) { - # Getting borrower details - my $memberdetails = GetMemberDetails($issue->[$i]{'borrowernumber'}); - $issue->[$i]{'borrowername'} = $memberdetails->{'firstname'} . " " . $memberdetails->{'surname'}; - $issue->[$i]{'cardnumber'} = $memberdetails->{'cardnumber'}; my $datedue = $issue->[$i]{'date_due'}; my $issuedate = $issue->[$i]{'issuedate'}; $issue->[$i]{'date_due'} = C4::Dates->new($issue->[$i]{'date_due'}, 'iso')->output('syspref'); @@ -306,7 +302,7 @@ sub build_issue_data { #find the charge for an item my ( $charge, $itemtype ) = - GetIssuingCharges( $issue->[$i]{'itemnumber'}, $borrowernumber ); + GetIssuingCharges( $issue->[$i]{'itemnumber'}, $issue->[$i]{'borrowernumber'} ); my $itemtypeinfo = getitemtypeinfo($itemtype); $row{'itemtype_description'} = $itemtypeinfo->{description}; @@ -314,7 +310,7 @@ sub build_issue_data { $row{'charge'} = sprintf( "%.2f", $charge ); - my ( $renewokay,$renewerror ) = CanBookBeRenewed( $borrowernumber, $issue->[$i]{'itemnumber'}, $override_limit ); + my ( $renewokay,$renewerror ) = CanBookBeRenewed( $issue->[$i]{'borrowernumber'}, $issue->[$i]{'itemnumber'}, $override_limit ); $row{'norenew'} = !$renewokay; $row{'can_confirm'} = ( !$renewokay && $renewerror ne 'on_reserve' ); $row{"norenew_reason_$renewerror"} = 1 if $renewerror; -- 1.7.2.5