From: Peter Rabbitson Date: Sat, 27 Nov 2010 23:14:22 +0000 (+0100) Subject: Fix count on rs with a having clause with an aliased condition X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=e493ecb281730e358d23551d4addc5157b850892;p=dbsrgits%2FDBIx-Class-Historic.git Fix count on rs with a having clause with an aliased condition --- diff --git a/Changes b/Changes index 95b2dde..33f24a5 100644 --- a/Changes +++ b/Changes @@ -28,6 +28,7 @@ Revision history for DBIx::Class thread support (still problematic, pending a DBI fix) - Properly quote table name on INSERT with no values - Work around possible Storage destruction warnings + - Fix count of grouped resultsets using HAVING with aliases * Misc - Switch all serialization to use Storable::nfreeze for portable diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 8aa1d5f..580ab20 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -1306,10 +1306,42 @@ sub _count_subq_rs { if (ref $sel eq 'HASH' and $sel->{-as}); } - for my $g_part (@$g) { - my $colpiece = $sel_index->{$g_part} || $g_part; + # anything from the original select mentioned on the group-by needs to make it to the inner selector + # also look for named aggregates referred in the having clause + # having often contains scalarrefs - thus parse it out entirely + my @parts = @$g; + if ($attrs->{having}) { + local $sql_maker->{having_bind}; + local $sql_maker->{quote_char} = $sql_maker->{quote_char}; + local $sql_maker->{name_sep} = $sql_maker->{name_sep}; + unless (defined $sql_maker->{quote_char} and length $sql_maker->{quote_char}) { + $sql_maker->{quote_char} = [ "\x00", "\xFF" ]; + # if we don't unset it we screw up retarded but unfortunately working + # 'MAX(foo.bar)' => { '>', 3 } + $sql_maker->{name_sep} = ''; + } + + my ($lquote, $rquote, $sep) = map { quotemeta $_ } ($sql_maker->_quote_chars, $sql_maker->name_sep); + + my $sql = $sql_maker->_parse_rs_attrs ({ having => $attrs->{having} }); + + # search for both a proper quoted qualified string, for a naive unquoted scalarref + # and if all fails for an utterly naive quoted scalar-with-function + while ($sql =~ / + $rquote $sep $lquote (.+?) $rquote + | + [\s,] \w+ \. (\w+) [\s,] + | + [\s,] $lquote (.+?) $rquote [\s,] + /gx) { + push @parts, ($1 || $2 || $3); # one of them matched if we got here + } + } + + for (@parts) { + my $colpiece = $sel_index->{$_} || $_; - # disqualify join-based group_by's. Arcane but possible query + # unqualify join-based group_by's. Arcane but possible query # also horrible horrible hack to alias a column (not a func.) # (probably need to introduce SQLA syntax) if ($colpiece =~ /\./ && $colpiece !~ /^$attrs->{alias}\./) { diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index 07fa550..61f8aac 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -259,7 +259,9 @@ sub _resolve_aliastypes_from_select_args { local $sql_maker->{name_sep} = $sql_maker->{name_sep}; unless (defined $sql_maker->{quote_char} and length $sql_maker->{quote_char}) { - $sql_maker->{quote_char} = "\x00"; + $sql_maker->{quote_char} = ["\x00", "\xFF"]; + # if we don't unset it we screw up retarded but unfortunately working + # 'MAX(foo.bar)' => { '>', 3 } $sql_maker->{name_sep} = ''; } diff --git a/t/count/count_rs.t b/t/count/count_rs.t index f947a9b..b2c2827 100644 --- a/t/count/count_rs.t +++ b/t/count/count_rs.t @@ -8,8 +8,6 @@ use DBICTest; use DBIC::SqlMakerTest; use DBIC::DebugObj; -plan tests => 10; - my $schema = DBICTest->init_schema(); # non-collapsing prefetch (no multi prefetches) @@ -115,3 +113,38 @@ my $schema = DBICTest->init_schema(); 'count_rs db-side limit applied', ); } + +# count with a having clause +{ + my $rs = $schema->resultset("Artist")->search( + {}, + { + join => 'cds', + group_by => 'me.artistid', + '+select' => [ { max => 'cds.year', -as => 'newest_cd_year' } ], + '+as' => ['newest_cd_year'], + having => { 'newest_cd_year' => '2001' } + } + ); + + my $crs = $rs->count_rs; + + is_same_sql_bind ( + $crs->as_query, + '(SELECT COUNT( * ) + FROM ( + SELECT me.artistid, MAX( cds.year ) AS newest_cd_year + FROM artist me + LEFT JOIN cd cds ON cds.artist = me.artistid + GROUP BY me.artistid + HAVING newest_cd_year = ? + ) me + )', + [ [ 'newest_cd_year' => '2001' ],], + 'count with having clause keeps sql as alias', + ); + + is ($crs->next, 2, 'Correct artist count (each with one 2001 cd)'); +} + +done_testing;