From 953a46a0888b38a3c34e1f01ee1a0e420d904273 Mon Sep 17 00:00:00 2001 From: Chris Nighswonger Date: Mon, 4 Aug 2008 10:15:19 -0500 Subject: [PATCH] kohabug 2417 Removing hardcoded query limit from reports 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 Signed-off-by: Joshua Ferraro --- C4/Reports.pm | 127 ++++++++++++------ .../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 6be341acba..811a9191fc 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( '', @data ); - $row = "$row"; - $temphash{'row'} = $row; - if ( $format eq 'text' ) { - $string .= "\n" . $row; - } - if ($format eq 'tab' ){ - $row = join("\t",@data); - $string .="\n" . $row; - } - if ($format eq 'csv' ){ - $row = join(",",@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( '', @data ); + $row = "$row"; + $temphash{'row'} = $row; + if ( $format eq 'text' ) { + $string .= "\n" . $row; + } + if ($format eq 'tab' ){ + $row = join("\t",@data); + $string .="\n" . $row; + } + if ($format eq 'csv' ){ + $row = join(",",@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 b9c01b041c..8bf572a2de 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" -->">

+ diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 793eaa2868..b7cb46d3bc 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; } -- 2.39.5