From: Matt S Trout Date: Wed, 22 Feb 2006 16:56:36 +0000 (+0000) Subject: Working HAVING, DBD::SQLite returns duff data so tests don't pass yet X-Git-Tag: v0.06000~61^2~16 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=8839560b9a5ec14ea4a921831a46962a2a74d87a;p=dbsrgits%2FDBIx-Class.git 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 --- 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 --- 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 --- 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',