Bugzilla – Attachment 11654 Details for
Bug 8594
A report containing a subquery that has a 'limit' will have that limit stripped out
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED-OFF] Bug 8594 - prevent the report system from breaking some subqueries
SIGNED-OFF-Bug-8594---prevent-the-report-system-fr.patch (text/plain), 7.53 KB, created by
Nicole C. Engard
on 2012-08-16 23:42:04 UTC
(
hide
)
Description:
[SIGNED-OFF] Bug 8594 - prevent the report system from breaking some subqueries
Filename:
MIME Type:
Creator:
Nicole C. Engard
Created:
2012-08-16 23:42:04 UTC
Size:
7.53 KB
patch
obsolete
>From d3c89079a622d72975280dfde2c2b2fe8cd24f46 Mon Sep 17 00:00:00 2001 >From: Robin Sheat <robin@catalyst.net.nz> >Date: Wed, 8 Aug 2012 18:02:13 +0200 >Subject: [PATCH] [SIGNED-OFF] Bug 8594 - prevent the report system from breaking some subqueries > >If you had a report query that had a subquery in the fields list, and >that subquery had a LIMIT specifier, then it would be removed which >could break your query. This patch prevents this case from breaking by >ensuring that only a LIMIT that follows the last WHERE in the query is >removed. > >If you don't have a WHERE, then it will behave like it always >did, removing all the cases of LIMIT (which would still break a subquery >but this is a) more rare, and b) would require more intelligent parsing >to deal with. > >Also adds test cases and function documentation. > >Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> > >Tested with this report: > >select biblionumber, (select itemnumber from items where items.biblionumber=biblio.biblionumber LIMIT 1) from biblio where biblionumber<1000; > >and it worked like a charm >--- > C4/Reports/Guided.pm | 43 ++++++++++++++++-- > t/db_dependent/Reports/Guided.t | 92 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 130 insertions(+), 5 deletions(-) > create mode 100644 t/db_dependent/Reports/Guided.t > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index fbc25d8..f682fe8 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -413,11 +413,44 @@ sub select_2_select_count ($) { > $sql =~ s/\bSELECT\W+(?:\w+\W+){1,}?FROM\b|\bSELECT\W\*\WFROM\b/SELECT count(*) FROM /ig; > return $sql; > } >-sub strip_limit ($) { >- my $sql = shift or return; >- ($sql =~ /\bLIMIT\b/i) or return ($sql, 0, undef); >- $sql =~ s/\bLIMIT\b\s*(\d+)(\s*\,\s*(\d+))?\s*/ /ig; >- return ($sql, (defined $2 ? $1 : 0), (defined $3 ? $3 : $1)); # offset can default to 0, LIMIT cannot! >+ >+# This removes the LIMIT from the query so that a custom one can be specified. >+# Usage: >+# ($new_sql, $offset, $limit) = strip_limit($sql); >+# >+# Where: >+# $sql is the query to modify >+# $new_sql is the resulting query >+# $offset is the offset value, if the LIMIT was the two-argument form, >+# 0 if it wasn't otherwise given. >+# $limit is the limit value >+# >+# Notes: >+# * This makes an effort to not break subqueries that have their own >+# LIMIT specified. It does that by only removing a LIMIT if it comes after >+# a WHERE clause (which isn't perfect, but at least should make more cases >+# work - subqueries with a limit in the WHERE will still break.) >+# * If your query doesn't have a WHERE clause then all LIMITs will be >+# removed. This may break some subqueries, but is hopefully rare enough >+# to not be a big issue. >+sub strip_limit { >+ my ($sql) = @_; >+ >+ return unless $sql; >+ return ($sql, 0, undef) unless $sql =~ /\bLIMIT\b/i; >+ >+ # Two options: if there's no WHERE clause in the SQL, we simply capture >+ # any LIMIT that's there. If there is a WHERE, we make sure that we only >+ # capture a LIMIT after the last one. This prevents stomping on subqueries. >+ if ($sql !~ /\bWHERE\b/i) { >+ (my $res = $sql) =~ s/\bLIMIT\b\s*(\d+)(\s*\,\s*(\d+))?\s*/ /ig; >+ return ($res, (defined $2 ? $1 : 0), (defined $3 ? $3 : $1)); >+ } else { >+ my $res = $sql; >+ $res =~ m/.*\bWHERE\b/gsi; >+ $res =~ s/\G(.*)\bLIMIT\b\s*(\d+)(\s*\,\s*(\d+))?\s*/$1 /is; >+ return ($res, (defined $3 ? $2 : 0), (defined $4 ? $4 : $2)); >+ } > } > > sub execute_query ($;$$$) { >diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t >new file mode 100644 >index 0000000..5963066 >--- /dev/null >+++ b/t/db_dependent/Reports/Guided.t >@@ -0,0 +1,92 @@ >+# Copyright 2012 Catalyst IT Ltd. >+# >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it under the >+# terms of the GNU General Public License as published by the Free Software >+# Foundation; either version 2 of the License, or (at your option) any later >+# version. >+# >+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY >+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR >+# A PARTICULAR PURPOSE. See the GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License along >+# with Koha; if not, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use strict; >+use warnings; >+ >+use Test::More tests => 19; # last test to print >+ >+use_ok('C4::Reports::Guided'); >+ >+# This is the query I found that triggered bug 8594. >+my $sql = "SELECT aqorders.ordernumber, biblio.title, biblio.biblionumber, items.homebranch, >+ aqorders.entrydate, aqorders.datereceived, >+ (SELECT DATE(datetime) FROM statistics >+ WHERE itemnumber=items.itemnumber AND >+ (type='return' OR type='issue') LIMIT 1) >+ AS shelvedate, >+ DATEDIFF(COALESCE( >+ (SELECT DATE(datetime) FROM statistics >+ WHERE itemnumber=items.itemnumber AND >+ (type='return' OR type='issue') LIMIT 1), >+ aqorders.datereceived), aqorders.entrydate) AS totaldays >+FROM aqorders >+LEFT JOIN biblio USING (biblionumber) >+LEFT JOIN items ON (items.biblionumber = biblio.biblionumber >+ AND dateaccessioned=aqorders.datereceived) >+WHERE (entrydate >= '2011-01-01' AND (datereceived < '2011-02-01' OR datereceived IS NULL)) >+ AND items.homebranch LIKE 'INFO' >+ORDER BY title"; >+ >+my ($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql); >+is($res_sql, $sql, "Not breaking subqueries"); >+is($res_lim1, 0, "Returns correct default offset"); >+is($res_lim2, undef, "Returns correct default LIMIT"); >+ >+# Now the same thing, but we want it to remove the LIMIT from the end >+ >+my $test_sql = $res_sql . " LIMIT 242"; >+($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql); >+# The replacement drops a ' ' where the limit was >+is(trim($res_sql), $sql, "Correctly removes only final LIMIT"); >+is($res_lim1, 0, "Returns correct default offset"); >+is($res_lim2, 242, "Returns correct extracted LIMIT"); >+ >+$test_sql = $res_sql . " LIMIT 13,242"; >+($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql); >+# The replacement drops a ' ' where the limit was >+is(trim($res_sql), $sql, "Correctly removes only final LIMIT (with offset)"); >+is($res_lim1, 13, "Returns correct extracted offset"); >+is($res_lim2, 242, "Returns correct extracted LIMIT"); >+ >+# After here is the simpler case, where there isn't a WHERE clause to worry >+# about. >+ >+# First case with nothing to change >+$sql = "SELECT * FROM items"; >+($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($sql); >+is($res_sql, $sql, "Not breaking simple queries"); >+is($res_lim1, 0, "Returns correct default offset"); >+is($res_lim2, undef, "Returns correct default LIMIT"); >+ >+$test_sql = $sql . " LIMIT 242"; >+($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql); >+is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case"); >+is($res_lim1, 0, "Returns correct default offset"); >+is($res_lim2, 242, "Returns correct extracted LIMIT"); >+ >+$test_sql = $sql . " LIMIT 13,242"; >+($res_sql, $res_lim1, $res_lim2) = C4::Reports::Guided::strip_limit($test_sql); >+is(trim($res_sql), $sql, "Correctly removes LIMIT in simple case (with offset)"); >+is($res_lim1, 13, "Returns correct extracted offset"); >+is($res_lim2, 242, "Returns correct extracted LIMIT"); >+ >+sub trim { >+ my ($s) = @_; >+ $s =~ s/^\s*(.*?)\s*$/$1/s; >+ return $s; >+} >-- >1.7.2.3
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8594
:
11453
| 11654