[Koha-patches] [PATCH] kohabug 2417 Removing hardcoded query limit from reports

Galen Charlton galen.charlton at liblime.com
Mon Aug 4 17:15:19 CEST 2008


From: Chris Nighswonger <chris.nighswonger at liblime.com>

This patch removes a hardcoded 'LIMIT 20' which was added to all report queries
thus limiting all reports to only the first twenty rows of applicable data. In
its place this patch introduces code to paginate through all applicable data,
regardless of how many rows are available. The code will also honor any user
defined 'LIMIT' in reports based on SQL entered directly by the user.

This patch also adds column labels to 'tab' and 'csv' files generated by reports.
NOTE: Only user defined 'LIMIT's apply to 'tab,' 'csv,' and 'text' files.

Signed-off-by: Galen Charlton <galen.charlton at liblime.com>
---
 C4/Reports.pm                                      |  127 +++++++++++++-------
 .../en/modules/reports/guided_reports_start.tmpl   |    1 +
 reports/guided_reports.pl                          |   58 ++++++----
 3 files changed, 122 insertions(+), 64 deletions(-)

diff --git a/C4/Reports.pm b/C4/Reports.pm
index 6be341a..811a919 100644
--- a/C4/Reports.pm
+++ b/C4/Reports.pm
@@ -25,6 +25,7 @@ use C4::Context;
 use C4::Output;
 use XML::Simple;
 use XML::Dumper;
+use C4::Debug;
 # use Smart::Comments;
 # use Data::Dumper;
 
@@ -328,18 +329,56 @@ sub get_criteria {
     return ( \@criteria_array );
 }
 
-sub execute_query {
-    my ( $sql, $type, $format, $id ) = @_;
-    my $dbh = C4::Context->dbh();
+=item execute_query
 
-    # take this line out when in production
-	if ($format eq 'csv' or $format eq 'tab'){
-		}
-	else {
-		$sql .= " LIMIT 20";
-	}
+=head2 ($results, $total) = execute_query($sql, $type, $offset, $limit, $format, $id)
+
+    When passed C<$sql>, this function returns an array ref containing a result set
+    suitably formatted for display in html or for output as a flat file when passed in
+    C<$format> and C<$id>.  Valid values for C<$format> are 'text,' 'tab,' 'csv,' or 'url.
+    C<$sql>, C<$type>, C<$offset>, and C<$limit> are required parameters. If a valid
+    C<$format> is passed in, C<$offset> and C<$limit> are ignored for obvious reasons.
+    A LIMIT specified by the user in a user-supplied SQL query WILL apply in any case.
+
+=cut
+
+sub execute_query ($$$$;$$) {
+    my ( $sql, $type, $offset, $limit, $format, $id ) = @_;
+    my @params;
+    my $total = 0;
+    my ($useroffset, $userlimit);
+    my $dbh = C4::Context->dbh();
+    unless ($format eq 'text' || $format eq 'tab' || $format eq 'csv' || $format eq 'url'){
+        # Grab offset/limit from user supplied LIMIT and drop the LIMIT so we can control pagination
+        if ($sql =~ /LIMIT/i) {
+            $sql =~ s/LIMIT\W?(\d+)?\,?\W+?(\d+)//ig;
+            $debug and warn "User has supplied LIMIT\n";
+            $useroffset = $1;
+            $userlimit = $2;
+            $debug and warn "User supplied offset = $useroffset, limit = $userlimit\n";
+            $offset += $useroffset if $useroffset;
+            # keep track of where we are if there is a user supplied LIMIT
+            if ( $offset + $limit > $userlimit ) {
+                $limit = $userlimit - $offset;
+            }
+        }
+        my $countsql = $sql;
+        $sql .= " LIMIT ?, ?";
+        $debug and warn "Passing query with params offset = $offset, limit = $limit\n";
+        @params = ($offset, $limit);
+        # Modify the query passed in to create a count query... (I think this covers all cases -crn)
+        $countsql =~ s/\bSELECT\W+(?:\w+\W+){1,}?FROM\b|\bSELECT\W\*\WFROM\b/SELECT count(*) FROM /ig;
+        $debug and warn "original query: $sql\n";
+        $debug and warn "count query: $countsql\n";
+        my $sth1 = $dbh->prepare($countsql);
+        $sth1->execute();
+        $total = $sth1->fetchrow();
+        $debug and warn "total records for this query: $total\n";
+        $total = $userlimit if defined($userlimit) and $userlimit < $total;     # we will never exceed a user defined LIMIT and...
+        $userlimit = $total if defined($userlimit) and $userlimit > $total;     # we will never exceed the total number of records available to satisfy the query
+    }
     my $sth = $dbh->prepare($sql);
-    $sth->execute();
+    $sth->execute(@params);
     my $colnames=$sth->{'NAME'};
     my @results;
     my $row;
@@ -349,6 +388,12 @@ sub execute_query {
     $temphash{'row'} = $row;
     push @results, \%temphash;
     my $string;
+    if ($format eq 'tab') {
+        $string = join("\t",@$colnames);
+    }
+    if ($format eq 'csv') {
+        $string = join(",",@$colnames);
+    }
     my @xmlarray;
     while ( my @data = $sth->fetchrow_array() ) {
         # if the field is a date field, it needs formatting
@@ -357,43 +402,41 @@ sub execute_query {
             my $date = C4::Dates->new($data, "iso");
             $data = $date->output();
         }
-            # tabular
-            my %temphash;
-            my $row = join( '</td><td>', @data );
-            $row = "<tr><td>$row</td></tr>";
-            $temphash{'row'} = $row;
-            if ( $format eq 'text' ) {
-                $string .= "\n" . $row;
-            }
-			if ($format eq 'tab' ){
-				$row = join("\t", at data);
-				$string .="\n" . $row;
-			}
-			if ($format eq 'csv' ){
-				$row = join(",", at data);
-				$string .="\n" . $row;
-			}
-		if ($format eq 'url'){
-			my $temphash;
-			@$temphash{@$colnames}=@data;
-			push @xmlarray,$temphash;
-		}
-            push @results, \%temphash;
-#        }
+        # tabular
+        my %temphash;
+        my $row = join( '</td><td>', @data );
+        $row = "<tr><td>$row</td></tr>";
+        $temphash{'row'} = $row;
+        if ( $format eq 'text' ) {
+            $string .= "\n" . $row;
+        }
+        if ($format eq 'tab' ){
+            $row = join("\t", at data);
+            $string .="\n" . $row;
+        }
+        if ($format eq 'csv' ){
+            $row = join(",", at data);
+            $string .="\n" . $row;
+        }
+        if ($format eq 'url'){
+            my $temphash;
+            @$temphash{@$colnames}=@data;
+            push @xmlarray,$temphash;
+        }
+        push @results, \%temphash;
     }
-    $sth->finish();
     if ( $format eq 'text' || $format eq 'tab' || $format eq 'csv' ) {
         return $string;
     }
-	elsif ($format eq 'url') {
-		my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id";
-		my $dump = new XML::Dumper;
-		my $xml = $dump->pl2xml( \@xmlarray );
-		store_results($id,$xml);
-		return $url;
-	}
+    elsif ($format eq 'url') {
+        my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id";
+        my $dump = new XML::Dumper;
+        my $xml = $dump->pl2xml( \@xmlarray );
+        store_results($id,$xml);
+        return $url;
+    }
     else {
-        return ( \@results );
+        return ( \@results, $total );
     }
 }
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl
index b9c01b0..8bf572a 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tmpl
@@ -359,6 +359,7 @@ NAME="name" -->"><!-- TMPL_VAR NAME="name"--></label></td><td>
 <!-- TMPL_IF NAME="execute" -->
 <h1><!-- TMPL_VAR NAME="name" --></h1>
 <p><!-- TMPL_VAR NAME="notes" --></p>
+<!-- TMPL_VAR name='pagination_bar'-->
 <table>
 <!-- TMPL_LOOP NAME="results" -->
 <!-- TMPL_VAR NAME="row" -->
diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl
index 793eaa2..b7cb46d 100755
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -23,6 +23,7 @@ use C4::Reports;
 use C4::Auth;
 use C4::Output;
 use C4::Dates qw( DHTMLcalendar );
+use C4::Debug;
 
 =head1 NAME
 
@@ -330,30 +331,43 @@ elsif ( $phase eq 'Save Report' ) {
 	);
 }
 
-elsif ( $phase eq 'Execute' ) {
-	# run the sql, and output results in a template	
-    my $sql     = $input->param('sql');
-    my $type    = $input->param('type');
-    my $results = execute_query($sql,$type);
-    $template->param(
-        'results' => $results,
-        'sql' => $sql,
-        'execute' => 1
-    );
-}
+# This condition is not used currently
+#elsif ( $phase eq 'Execute' ) {
+#    # run the sql, and output results in a template	
+#    my $sql     = $input->param('sql');
+#    my $type    = $input->param('type');
+#    my $results = execute_query($sql, $type);
+#    $template->param(
+#        'results' => $results,
+#        'sql' => $sql,
+#        'execute' => 1,
+#    );
+#}
 
 elsif ($phase eq 'Run this report'){
     # execute a saved report
-	my $report = $input->param('reports');
-	my ($sql,$type,$name,$notes) = get_saved_report($report);
-	my $results = execute_query($sql,$type);
-        $template->param(
-            'results' => $results,
-            'sql' => $sql,
-            'execute' => 1,
-            'name' => $name,
-            'notes' => $notes,
-        );
+    # FIXME: The default limit should not be hardcoded...
+    my $limit = 20;
+    my $offset;
+    my $report = $input->param('reports');
+    # offset algorithm
+    if ($input->param('page')) {
+        $offset = ($input->param('page') - 1) * 20;
+    } else {
+        $offset = 0;
+    }
+    my ($sql,$type,$name,$notes) = get_saved_report($report);
+    my ($results, $total) = execute_query($sql, $type, $offset, $limit);
+    my $totpages = int($total/$limit) + (($total % $limit) > 0 ? 1 : 0);
+    my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report&phase=Run%20this%20report";
+    $template->param(
+        'results'       => $results,
+        'sql'           => $sql,
+        'execute'       => 1,
+        'name'          => $name,
+        'notes'         => $notes,
+        'pagination_bar' => pagination_bar($url, $totpages, $input->param('page'), "page"),
+    );
 }	
 
 elsif ($phase eq 'Export'){
@@ -363,7 +377,7 @@ elsif ($phase eq 'Export'){
 	print $input->header(   -type => 'application/octet-stream',
 	          -attachment=>'reportresults.csv');
 	my $format=$input->param('format');
-	my $results = execute_query($sql,1,$format);
+	my $results = execute_query($sql,1,0,0,$format);
 	print $results;
 	
 }
-- 
1.5.5.GIT



More information about the Koha-patches mailing list