Working HAVING, DBD::SQLite returns duff data so tests don't pass yet

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 0056abd..09b7868 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -160,6 +160,7 @@ sub search {
   if( @_ ) {
     
     my $attrs = { %{$self->{attrs}} };
+    my $having = delete $attrs->{having};
     if (@_ > 1 && ref $_[$#_] eq 'HASH') {
      $attrs = { %$attrs, %{ pop(@_) } };
     }
@@ -174,6 +175,15 @@ sub search {
       $attrs->{where} = $where;
     }
 
+    if (defined $having) {
+      $having = (defined $attrs->{having}
+                ? { '-and' =>
+                    [ map { ref $_ eq 'ARRAY' ? [ -or => $_ ] : $_ }
+                        $having, $attrs->{having} ] }
+                : $having);
+      $attrs->{having} = $having;
+    }
+
     $rs = (ref $self)->new($self->result_source, $attrs);
   }
   else {
@@ -485,13 +495,15 @@ sub count {
       if @{ $self->get_cache };
     my $group_by;
     my $select = { 'count' => '*' };
-    if( $group_by = delete $self->{attrs}{group_by} ) {
+    my $attrs = { %{ $self->{attrs} } };
+    if( $group_by = delete $attrs->{group_by} ) {
+      delete $attrs->{having};
       my @distinct = (ref $group_by ?  @$group_by : ($group_by));
       # todo: try CONCAT for multi-column pk
       my @pk = $self->result_source->primary_columns;
       if( scalar(@pk) == 1 ) {
         my $pk = shift(@pk);
-        my $alias = $self->{attrs}{alias};
+        my $alias = $attrs->{alias};
         my $re = qr/^($alias\.)?$pk$/;
         foreach my $column ( @distinct) {
           if( $column =~ $re ) {
@@ -505,14 +517,12 @@ sub count {
       #use Data::Dumper; die Dumper $select;
     }
 
-    my $attrs = { %{ $self->{attrs} },
-                  select => $select,
-                  as => [ 'count' ] };
+    $attrs->{select} = $select;
+    $attrs->{as} = [ 'count' ];
     # offset, order by and page are not needed to count. record_filter is cdbi
     delete $attrs->{$_} for qw/rows offset order_by page pager record_filter/;
         
     ($self->{count}) = (ref $self)->new($self->result_source, $attrs)->cursor->next;
-    $self->{attrs}{group_by} = $group_by;
   }
   return 0 unless $self->{count};
   my $count = $self->{count};

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 8bfd500..492cc91 100644 (file)
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -17,8 +17,10 @@ use base qw/SQL::Abstract::Limit/;
 sub select {
   my ($self, $table, $fields, $where, $order, @rest) = @_;
   @rest = (-1) unless defined $rest[0];
-  $self->SUPER::select($table, $self->_recurse_fields($fields), 
-                         $where, $order, @rest);
+  local $self->{having_bind} = [];
+  my ($sql, @ret) = $self->SUPER::select($table,
+                      $self->_recurse_fields($fields), $where, $order, @rest);
+  return wantarray ? ($sql, @ret, @{$self->{having_bind}}) : $sql;
 }
 
 sub _emulate_limit {
@@ -49,11 +51,18 @@ sub _recurse_fields {
 sub _order_by {
   my $self = shift;
   my $ret = '';
+  my @extra;
   if (ref $_[0] eq 'HASH') {
     if (defined $_[0]->{group_by}) {
       $ret = $self->_sqlcase(' group by ')
                .$self->_recurse_fields($_[0]->{group_by});
     }
+    if (defined $_[0]->{having}) {
+      my $frag;
+      ($frag, @extra) = $self->_recurse_where($_[0]->{having});
+      push(@{$self->{having_bind}}, @extra);
+      $ret .= $self->_sqlcase(' having ').$frag;
+    }
     if (defined $_[0]->{order_by}) {
       $ret .= $self->SUPER::_order_by($_[0]->{order_by});
     }
@@ -395,8 +404,9 @@ sub _select {
   if (ref $condition eq 'SCALAR') {
     $order = $1 if $$condition =~ s/ORDER BY (.*)$//i;
   }
-  if (exists $attrs->{group_by}) {
+  if (exists $attrs->{group_by} || $attrs->{having}) {
     $order = { group_by => $attrs->{group_by},
+               having => $attrs->{having},
                ($order ? (order_by => $order) : ()) };
   }
   my @args = ('select', $attrs->{bind}, $ident, $select, $condition, $order);

diff --git a/t/run/16joins.tl b/t/run/16joins.tl
index 5ea8749..bfab938 100644 (file)
--- a/t/run/16joins.tl
+++ b/t/run/16joins.tl
@@ -7,7 +7,7 @@ BEGIN {
     eval "use DBD::SQLite";
     plan $@
         ? ( skip_all => 'needs DBD::SQLite for testing' )
-        : ( tests => 41 );
+        : ( tests => 42 );
 }
 
 # figure out if we've got a version of sqlite that is older than 3.2.6, in
@@ -253,24 +253,16 @@ SKIP: {
     cmp_ok( $rs->count, '==', 3, "count() ok after group_by on related column" );
 }
 
-$rs = $schema->resultset("CD")->search(
+$rs = $schema->resultset("Artist")->search(
   {},
-      { join => [qw/ artist /], group_by => [qw/ artist.name /], having =>{ 'MAX(cd.id)'=>{'<',5 } } }
+      { join => [qw/ cds /], group_by => [qw/ me.name /], having =>{ 'MAX(cds.cdid)'=>{'<',5 } } }
 );
 
-SKIP: {
-    skip "SQLite < 3.2.6 doesn't understand COUNT(DISTINCT())", 1
-        if $is_broken_sqlite;
-    cmp_ok( $rs->count, '==', 2, "count() ok after group_by on related column with a having" );
-}
+cmp_ok( $rs->all, '==', 2, "results ok after group_by on related column with a having" );
 
-$rs = $rs->search( {1=>1}, {  having =>{ 'count(*)'=>{'>',2 } }});
+$rs = $rs->search( undef, {  having =>{ 'count(*)'=>{'>',2 } }});
 
-SKIP: {
-    skip "SQLite < 3.2.6 doesn't understand COUNT(DISTINCT())", 1
-        if $is_broken_sqlite;
-    cmp_ok( $rs->count, '==', 1, "count() ok after group_by on related column with a having" );
-}
+cmp_ok( $rs->all, '==', 1, "count() ok after group_by on related column with a having" );
 
 $rs = $schema->resultset("Artist")->search(
         { 'cds.title' => 'Spoonful of bees',