Fix count on rs with a having clause with an aliased condition
Peter Rabbitson [Sat, 27 Nov 2010 23:14:22 +0000 (00:14 +0100)]
Changes
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/Storage/DBIHacks.pm
t/count/count_rs.t

diff --git a/Changes b/Changes
index 95b2dde..33f24a5 100644 (file)
--- 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
index 8aa1d5f..580ab20 100644 (file)
@@ -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}\./) {
index 07fa550..61f8aac 100644 (file)
@@ -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} = '';
   }
 
index f947a9b..b2c2827 100644 (file)
@@ -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;