Attachment 41775 Details for Bug 14704 – Bug 14704 - members/statistics.pl crashes because the SQL 'return'-keyword might be reserved at 'return FROM ('

[patch] Bug 14704 - members/statistics.pl crashes because the SQL 'return'-keyword might be reserved at 'return FROM ('

Bug-14704---membersstatisticspl-crashes-because-th.patch (text/plain), 5.02 KB, created by Olli-Antti Kivilahti on 2015-08-21 10:44:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Olli-Antti Kivilahti

Created: 2015-08-21 10:44:46 UTC

Size: 5.02 KB

patch

obsolete

>From 2cbc0daccb138c275d5ecdad135646559e3021c7 Mon Sep 17 00:00:00 2001
>From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
>Date: Fri, 21 Aug 2015 13:18:56 +0300
>Subject: [PATCH] Bug 14704 - members/statistics.pl crashes because the SQL
> 'return'-keyword might be reserved at 'return FROM ('
>
>At Ubuntu 14.04 using MariaDB, the following SQL doesn't work:
>
>SELECT COUNT(*) as count_precedent_state,issues,onloan,return
>FROM (
>    SELECT it.* FROM issues i, items it
>    WHERE i.borrowernumber = 1234 AND
>          i.itemnumber = it.itemnumber AND
>          DATE(i.issuedate) < CAST(now() AS date)
>    UNION
>    SELECT it.* FROM old_issues oi, items it
>    WHERE oi.borrowernumber = 1234 AND
>          oi.itemnumber = it.itemnumber AND
>          DATE(oi.issuedate) < CAST(now() AS date) AND
>          DATE(oi.returndate) = CAST(now() AS date)
>    ) tmp
>GROUP BY issues,onloan,return;
>
>We get "Syntax error near 'return FROM ('"
>
>This patch changes the SQL to prefix the selected columns with a table name prefix, so we get:
>
>SELECT COUNT(*) as count_precedent_state,tmp.issues,tmp.onloan,tmp.return
>FROM (
>    SELECT it.* FROM issues i, items it
>    WHERE i.borrowernumber = 1234 AND
>          i.itemnumber = it.itemnumber AND
>          DATE(i.issuedate) < CAST(now() AS date)
>    UNION
>    SELECT it.* FROM old_issues oi, items it
>    WHERE oi.borrowernumber = 1234 AND
>          oi.itemnumber = it.itemnumber AND
>          DATE(oi.issuedate) < CAST(now() AS date) AND
>          DATE(oi.returndate) = CAST(now() AS date)
>    ) tmp
>GROUP BY tmp.issues,tmp.onloan,tmp.return;
>
>Which is more formal and less implicit.
>
>TEST PLAN:
>1. Find a borrower who is an active library service consumer.
>2. Go to members/statistics.pl
>3. Observe normal behaviour, ie. no crashes.
>---
> C4/Members/Statistics.pm |   27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
>diff --git a/C4/Members/Statistics.pm b/C4/Members/Statistics.pm
>index 9f26b86..f985363 100644
>--- a/C4/Members/Statistics.pm
>+++ b/C4/Members/Statistics.pm
>@@ -42,6 +42,7 @@ BEGIN {
> 
> 
> our $fields = get_fields();
>+our $tmpSelectAlias = 'tmp';
> 
> sub get_fields {
>     my $r = C4::Context->preference('StatisticsFields') || 'location|itype|ccode';
>@@ -59,14 +60,22 @@ sub get_fields {
> sub construct_query {
>     my $count    = shift;
>     my $subquery = shift;
>-    my @select_fields = split '\|', $fields;
>+
>+    my @select_fields;
>+    #Some DB engines and/or MySQL variants cannot cope with return-keyword, thus it needs to be prefixed.
>+    if ($subquery =~ /$tmpSelectAlias$/) {
>+        @select_fields = map {"$tmpSelectAlias.$_"} split('\|', $fields);
>+    }
>+    else {
>+        @select_fields = map {($_ =~ /return/) ? "i.$_" : $_} split('\|', $fields);
>+    }
>+
>     my $query = "SELECT COUNT(*) as count_$count,";
>     $query .= join ',', @select_fields;
> 
>     $query .= " " . $subquery;
> 
>-    $fields =~ s/\|/,/g;
>-    $query .= " GROUP BY $fields;";
>+    $query .= " GROUP BY ".join(',', @select_fields);
> 
>     return $query;
> 
>@@ -81,10 +90,10 @@ sub GetTotalIssuesTodayByBorrower {
> 
>     my $query = construct_query "total_issues_today",
>         "FROM (
>-            SELECT it.* FROM issues i, items it WHERE i.itemnumber = it.itemnumber AND i.borrowernumber = ? AND DATE(i.issuedate) = CAST(now() AS date)
>+            SELECT it.*, i.return FROM issues i, items it WHERE i.itemnumber = it.itemnumber AND i.borrowernumber = ? AND DATE(i.issuedate) = CAST(now() AS date)
>             UNION
>-            SELECT it.* FROM old_issues oi, items it WHERE oi.itemnumber = it.itemnumber AND oi.borrowernumber = ? AND DATE(oi.issuedate) = CAST(now() AS date)
>-        ) tmp";     # alias is required by MySQL
>+            SELECT it.*, oi.return FROM old_issues oi, items it WHERE oi.itemnumber = it.itemnumber AND oi.borrowernumber = ? AND DATE(oi.issuedate) = CAST(now() AS date)
>+        ) $tmpSelectAlias";     # alias is required by MySQL
> 
>     my $sth = $dbh->prepare($query);
>     $sth->execute($borrowernumber, $borrowernumber);
>@@ -114,10 +123,10 @@ sub GetPrecedentStateByBorrower {
> 
>     my $query = construct_query "precedent_state",
>         "FROM (
>-            SELECT it.* FROM issues i, items it WHERE i.borrowernumber = ? AND i.itemnumber = it.itemnumber AND DATE(i.issuedate) < CAST(now() AS date)
>+            SELECT it.*, i.return FROM issues i, items it WHERE i.borrowernumber = ? AND i.itemnumber = it.itemnumber AND DATE(i.issuedate) < CAST(now() AS date)
>             UNION
>-            SELECT it.* FROM old_issues oi, items it WHERE oi.borrowernumber = ? AND oi.itemnumber = it.itemnumber AND DATE(oi.issuedate) < CAST(now() AS date) AND DATE(oi.returndate) = CAST(now() AS date)
>-        ) tmp";     # alias is required by MySQL
>+            SELECT it.*, oi.return FROM old_issues oi, items it WHERE oi.borrowernumber = ? AND oi.itemnumber = it.itemnumber AND DATE(oi.issuedate) < CAST(now() AS date) AND DATE(oi.returndate) = CAST(now() AS date)
>+        ) $tmpSelectAlias";     # alias is required by MySQL
> 
>     my $sth = $dbh->prepare($query);
>     $sth->execute($borrowernumber, $borrowernumber);
>-- 
>1.7.9.5

Actions: View | Diff | Splinter Review

Attachments on bug 14704: 41775