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