[Koha-patches] [PATCH] [Signed Off] Bug 6801: checkoverdues returns unnecessary fields, causing slowness
Liz Rea
lrea at nekls.org
Thu Sep 22 17:28:27 CEST 2011
From: Ian Walls <ian.walls at bywatersolutions.com>
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 <lrea at nekls.org>
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 <span class="ex">[% message %]</span>
<td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% todayissue.biblionumber %]&type=intra"><strong>[% todayissue.title |html %]</strong></a>[% IF ( todayissue.author ) %], by [% todayissue.author %][% END %][% IF ( todayissue.itemnotes ) %]- <span class="circ-hlt">[% todayissue.itemnotes %]</span>[% END %] <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% todayissue.biblionumber %]&itemnumber=[% todayissue.itemnumber %]#item[% todayissue.itemnumber %]">[% todayissue.barcode %]</a></td>
<td>[% UNLESS ( noItemTypeImages ) %] [% IF ( todayissue.itemtype_image ) %]<img src="[% todayissue.itemtype_image %]" alt="" />[% END %][% END %][% todayissue.itemtype %]</td>
<td>[% todayissue.checkoutdate %]</td>
- [% IF ( todayissue.multiple_borrowers ) %]<td>[% todayissue.borrowername %]</td>[% END %]
+ [% IF ( todayissue.multiple_borrowers ) %]<td>[% todayissue.firstname %] [% todayissue.surname %]</td>[% END %]
<td>[% todayissue.issuingbranchname %]</td>
<td>[% todayissue.itemcallnumber %]</td>
<td>[% todayissue.charge %]</td>
@@ -776,7 +776,7 @@ No patron matched <span class="ex">[% message %]</span>
[% previssue.itemtype %]
</td>
<td>[% previssue.displaydate %]</td>
- [% IF ( previssue.multiple_borrowers ) %]<td>[% previssue.borrowername %]</td>[% END %]
+ [% IF ( previssue.multiple_borrowers ) %]<td>[% previssue.firstname %] [% previssue.surname %]</td>[% END %]
<td>[% previssue.issuingbranchname %]</td>
<td>[% previssue.itemcallnumber %]</td>
<td>[% previssue.charge %]</td>
@@ -885,7 +885,7 @@ No patron matched <span class="ex">[% message %]</span>
<td>[% relissue.issuingbranchname %]</td>
<td>[% relissue.itemcallnumber %]</td>
<td>[% relissue.charge %]</td>
- <td>[% relissue.replacementprice %]</td><td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissue.borrowernumber %]">[% relissue.borrowername %] ([% relissue.cardnumber %])</a></td>
+ <td>[% relissue.replacementprice %]</td><td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissue.borrowernumber %]">[% relissue.firstname %] [% relissue.surname %] ([% relissue.cardnumber %])</a></td>
</tr>
[% END %] <!-- /loop relissues -->
<!-- /if relissues -->[% END %]
@@ -905,10 +905,10 @@ No patron matched <span class="ex">[% message %]</span>
<td>[% relprevissue.displaydate %]</td>
<td>[% relprevissue.issuingbranchname %]</td>
<td>[% relprevissue.itemcallnumber %]</td>
- [% IF ( relprevissue.multiple_borrowers ) %]<td>[% relprevissue.borrowername %]</td>[% END %]
+ [% IF ( relprevissue.multiple_borrowers ) %]<td>[% relprevissue.firstname %] [% relprevissue.surname %]</td>[% END %]
<td>[% relprevissue.charge %]</td>
<td>[% relprevissue.replacementprice %]</td>
- <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relprevissue.borrowernumber %]">[% relprevissue.borrowername %] ([% relprevissue.cardnumber %])</a></td>
+ <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relprevissue.borrowernumber %]">[% relprevissue.firstname %] [% relprevissue.surname %] ([% relprevissue.cardnumber %])</a></td>
</tr>
<!-- /loop relprevissue -->[% 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) {
<td>[% relissueloo.itemcallnumber %]</td>
<td>[% relissueloo.charge %]</td>
<td>[% relissueloo.replacementprice %]</td>
- <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissueloo.borrowernumber %]">[% relissueloo.borrowername %] ([% relissueloo.cardnumber %])</a></td>
+ <td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissueloo.borrowernumber %]">[% relissueloo.firstname %] [% relissueloo.surname %] ([% relissueloo.cardnumber %])</a></td>
</tr>
[% END %]
</tbody>
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
More information about the Koha-patches
mailing list