From f2253158d795fb2fe47061d6235d3ff773eeed5c Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 4 Aug 2021 08:59:40 +0200 Subject: [PATCH] Bug 28804: (bug 25026 follow-up) Handle SQL errors in reports Since bug 25026 DBMS errors are raised, but the report module is not dealing correctly with the errors. If an error occurred in execute_query, next queries will fail as well, we should skip them. Test plan: 1. Create report from SQL queries, containing errors (invalid syntax, etc.) 'SELECT id FROM borrowers' can do it 2. Execute the query => Without this patch you get a 500 => With this patch applied you see that the error raised at DBMS level is propagated to the UI 3. Confirm that there is no regression on valid queries Signed-off-by: Owen Leonard Signed-off-by: Katrin Fischer Signed-off-by: Jonathan Druart --- reports/guided_reports.pl | 53 +++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 77c19b3cb4..4c926b3c0a 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -822,10 +822,11 @@ elsif ($phase eq 'Run this report'){ my ($sql,$header_types) = $report->prep_report( \@param_names, \@sql_params ); $template->param(header_types => $header_types); my ( $sth, $errors ) = execute_query( $sql, $offset, $limit, undef, $report_id ); - my $total = nb_rows($sql) || 0; - unless ($sth) { + my $total; + if (!$sth) { die "execute_query failed to return sth for report $report_id: $sql"; - } else { + } elsif ( !$errors ) { + $total = nb_rows($sql) || 0; my $headers = header_cell_loop($sth); $template->param(header_row => $headers); while (my $row = $sth->fetchrow_arrayref()) { @@ -839,31 +840,33 @@ elsif ($phase eq 'Run this report'){ push @allrows, { cells => \@cells }; } } - } - my $totpages = int($total/$limit) + (($total % $limit) > 0 ? 1 : 0); - my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report_id&phase=Run%20this%20report&limit=$limit&want_full_chart=$want_full_chart"; - if (@param_names) { - $url = join('&param_name=', $url, map { URI::Escape::uri_escape_utf8($_) } @param_names); - } - if (@sql_params) { - $url = join('&sql_params=', $url, map { URI::Escape::uri_escape_utf8($_) } @sql_params); - } + my $totpages = int($total/$limit) + (($total % $limit) > 0 ? 1 : 0); + my $url = "/cgi-bin/koha/reports/guided_reports.pl?reports=$report_id&phase=Run%20this%20report&limit=$limit&want_full_chart=$want_full_chart"; + if (@param_names) { + $url = join('&param_name=', $url, map { URI::Escape::uri_escape_utf8($_) } @param_names); + } + if (@sql_params) { + $url = join('&sql_params=', $url, map { URI::Escape::uri_escape_utf8($_) } @sql_params); + } + $template->param( + 'results' => \@rows, + 'allresults' => \@allrows, + 'pagination_bar' => pagination_bar($url, $totpages, scalar $input->param('page')), + 'unlimited_total' => $total, + ); + } $template->param( - 'results' => \@rows, - 'allresults' => \@allrows, - 'sql' => $sql, - original_sql => $original_sql, - 'id' => $report_id, - 'execute' => 1, - 'name' => $name, - 'notes' => $notes, - 'errors' => defined($errors) ? [ $errors ] : undef, - 'pagination_bar' => pagination_bar($url, $totpages, scalar $input->param('page')), - 'unlimited_total' => $total, - 'sql_params' => \@sql_params, - 'param_names' => \@param_names, + 'sql' => $sql, + original_sql => $original_sql, + 'id' => $report_id, + 'execute' => 1, + 'name' => $name, + 'notes' => $notes, + 'errors' => defined($errors) ? [$errors] : undef, + 'sql_params' => \@sql_params, + 'param_names' => \@param_names, ); } } -- 2.39.5