From: Peter Rabbitson 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-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=0542ec57;p=dbsrgits%2FDBIx-Class-Historic.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;