From a043dc485269108ed2e8d72c8834c5852dbdd862 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Sat, 29 Jun 2013 20:46:51 -0400 Subject: [PATCH] Bug 7785 - MySQL-specific syntax in C4::Members columns() The initial thought was to remove this function. However, tools/import_borrowers.pl uses it. So rather than remove it to solve the problem, it was reworked to a more generic solution which runs faster. By accessing $sth->{NAME} directly, the driver becomes responsible for filling it correctly. This happens when a SELECT is done on the borrowers table. Since not all RDBMS's have "LIMIT 1", C4::Context->db_scheme2dbi was called to determine if the LIMIT clause can be used. If ever the backend changes from MySQL to something else, that module should be upgraded too. The columns method could be more generic and used elsewhere too. Comparison between the old method and the STH method showed a difference of 35 seconds for 40k iterations versus 19 seconds for the same amount of iterations on regardless of patrons when using a LIMIT clause. Failure to LIMIT the SELECT can lead to terrible performance, and should be remembered when adding a different backend. --- C4/Members.pm | 62 ++++++++++++++++++++++++++++++++++++++++++++-- t/db_dependent/Members.t | 5 +++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 3221f53..eecddab 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -695,10 +695,68 @@ sub GetMemberIssuesAndFines { return ($overdue_count, $issue_count, $total_fines); } -sub columns(;$) { - return @{C4::Context->dbh->selectcol_arrayref("SHOW columns from borrowers")}; + +=head2 columns + + C4::Member->columns + +=head3 USAGE + + use C4::Member; + my @borrower_columns = C4::Member->columns; + +=head3 RETURNS + + The array of borrowers' table columns on success. + An empty array on failure. + +=head3 NOTES + + This runs significantly faster than the previous code while + being mostly SQL-agnostic. + +=cut + +sub columns { + + # Check what DB we are using (current only returns mysql) + my $db=C4::Context->db_scheme2dbi(C4::Context->config("db_scheme")); + + # FIXME: Other DB's may require an additional elsif, + # or revision to the if. :) + # Not limiting the SQL query causes a drastic drop + # in speed (~3-4 seconds for ~1 million records + # compared to <1s with limit 1). + my $sql; + if ( 'mysql,' =~ /$db/ ) { + $sql = 'SELECT * FROM borrowers LIMIT 1;'; + } + else { + $sql = 'SELECT * FROM borrowers;'; + } + + # Get the database handle. + my $dbh = C4::Context->dbh; + + # Run the SQL statement to load STH's readonly properties. + my $sth = $dbh->prepare($sql); + my $rv = $sth->execute(); + + # This only fails if the table doesn't exist. + # This will always be called AFTER an install or upgrade, + # so borrowers will exist! + my @data; + if ($sth->{NUM_OF_FIELDS}>0) { + @data = @{$sth->{NAME}}; + } + else { + @data = (); + } + $sth->finish(); + return @data; } + =head2 ModMember my $success = ModMember(borrowernumber => $borrowernumber, diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t index 634ac0c..d2ff01b 100755 --- a/t/db_dependent/Members.t +++ b/t/db_dependent/Members.t @@ -6,7 +6,7 @@ use strict; use warnings; -use Test::More tests => 22; +use Test::More tests => 23; use Data::Dumper; BEGIN { @@ -169,6 +169,9 @@ C4::Context->clear_syspref_cache(); my $notice_email = GetNoticeEmailAddress($member->{'borrowernumber'}); is ($notice_email, $EMAILPRO, "GetNoticeEmailAddress returns correct value when AutoEmailPrimaryAddress is emailpro"); +my @borrowers_columns = C4::Members->columns; +ok ( $#borrowers_columns > 1, "C4::Member->column returned a reasonable number of columns (" . ($#borrowers_columns + 1) . ")") + or diag("WARNING: Check that the borrowers table exists and has the correct fields defined."); # clean up DelMember($member->{borrowernumber}); -- 1.7.9.5