From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 5 Jan 2011 14:43:15 +0000 (+0100)
Subject: Fix bind transport for group_by (this code is so fucking ugly...)
X-Git-Tag: v0.08127~33
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=0542ec57;p=dbsrgits%2FDBIx-Class.git

Fix bind transport for group_by (this code is so fucking ugly...)
---

diff --git a/Changes b/Changes
index 13171b6..eaee66e 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 Revision history for DBIx::Class
 
+    * Fixes
+        - Fix proper composition of bind values across all possible
+          SQL areas ( group_by => \[ ... ] now works properly )
+
 0.08126 2010-12-28 18:10 (UTC)
     * Fixes
         - Bump forgotten Class::Accessor::Grouped core dependency
diff --git a/lib/DBIx/Class/SQLMaker.pm b/lib/DBIx/Class/SQLMaker.pm
index 322ad33..cb9dcd8 100644
--- a/lib/DBIx/Class/SQLMaker.pm
+++ b/lib/DBIx/Class/SQLMaker.pm
@@ -215,7 +215,7 @@ sub select {
 
 sub _assemble_binds {
   my $self = shift;
-  return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/select from where having order/);
+  return map { @{ (delete $self->{"${_}_bind"}) || [] } } (qw/select from where group having order/);
 }
 
 my $for_syntax = {
@@ -317,8 +317,13 @@ sub _parse_rs_attrs {
 
   my $sql = '';
 
-  if (my $g = $self->_recurse_fields($arg->{group_by}) ) {
-    $sql .= $self->_sqlcase(' group by ') . $g;
+  if ($arg->{group_by}) {
+    # horible horrible, waiting for refactor
+    local $self->{select_bind};
+    if (my $g = $self->_recurse_fields($arg->{group_by}) ) {
+      $sql .= $self->_sqlcase(' group by ') . $g;
+      push @{$self->{group_bind} ||= []}, @{$self->{select_bind}||[]};
+    }
   }
 
   if (defined $arg->{having}) {
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index ba1faa4..864cbf5 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -262,8 +262,10 @@ sub _resolve_aliastypes_from_select_args {
   my $sql_maker = $self->sql_maker;
 
   # these are throw away results, do not pollute the bind stack
-  local $sql_maker->{having_bind};
   local $sql_maker->{select_bind};
+  local $sql_maker->{where_bind};
+  local $sql_maker->{group_bind};
+  local $sql_maker->{having_bind};
 
   # we can't scan properly without any quoting (\b doesn't cut it
   # everywhere), so unless there is proper quoting set - use our
diff --git a/t/sqlmaker/bind_transport.t b/t/sqlmaker/bind_transport.t
new file mode 100644
index 0000000..98baa4f
--- /dev/null
+++ b/t/sqlmaker/bind_transport.t
@@ -0,0 +1,59 @@
+use strict;
+use warnings;
+use Test::More;
+
+use lib qw(t/lib);
+use DBICTest;
+use DBIC::SqlMakerTest;
+
+my $schema = DBICTest->init_schema();
+
+my $ne_bind = [ _ne => 'bar' ];
+my $rs = $schema->resultset('CD')->search({ -and => [
+  'me.artist' => { '!=', 'foo' },
+  'me.artist' => { '!=', \[ '?', $ne_bind ] },
+]});
+
+# bogus sql query to make sure bind composition happens properly
+my $complex_rs = $rs->search({}, {
+  '+columns' => { cnt => $rs->count_rs->as_query },
+  '+select' => \[ 'me.artist + ?', [ _add => 1 ] ], # free select
+  group_by => ['me.cdid', \[ 'me.artist - ?', [ _sub => 2 ] ] ],
+  having => \[ 'me.artist < ?', [ _lt => 3 ] ],
+  order_by => \[ 'me.artist * ? ', [ _mu => 4 ] ],
+  rows => 1,
+  page => 3,
+});
+
+for (1,2) {
+  is_same_sql_bind (
+    $complex_rs->as_query,
+    '(
+      SELECT  me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track,
+              (SELECT COUNT( * ) FROM cd me WHERE me.artist != ? AND me.artist != ?),
+              me.artist + ?
+        FROM cd me
+      WHERE me.artist != ? AND me.artist != ?
+      GROUP BY me.cdid, me.artist - ?
+      HAVING me.artist < ?
+      ORDER BY me.artist * ?
+      LIMIT 1 OFFSET 2
+    )',
+    [
+      [ 'me.artist' => 'foo' ],
+      $ne_bind,
+      [ _add => 1 ],
+      [ 'me.artist' => 'foo' ],
+      $ne_bind,
+      [ _sub => 2 ],
+      [ _lt => 3 ],
+      [ _mu => 4 ],
+    ],
+    'Correct crazy sql',
+  );
+}
+
+# see if we get anything back at all
+isa_ok ($complex_rs->next, 'DBIx::Class::Row');
+
+done_testing;