From: Justin Hunter Date: Sat, 14 Mar 2009 14:33:03 +0000 (+0000) Subject: * change count with group_by (distinct) to use a subquery X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=2a2ca43fa8c62a99c36c983b1af3b31e2375f0ac;p=dbsrgits%2FDBIx-Class-Historic.git * change count with group_by (distinct) to use a subquery * rewrite of _bind_to_sql (uses placeholders and bindvars) * tests for count distinct * fixed tests for from subquery --- diff --git a/lib/DBIx/Class.pm b/lib/DBIx/Class.pm index bb73581..b0612ae 100644 --- a/lib/DBIx/Class.pm +++ b/lib/DBIx/Class.pm @@ -207,6 +207,8 @@ andyg: Andy Grundman ank: Andres Kievsky +arcanez: Justin Hunter + ash: Ash Berlin bert: Norbert Csongradi diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index dd911ad..25d91e9 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -1143,11 +1143,11 @@ sub count { sub _count { # Separated out so pager can get the full count my $self = shift; - my $select = { count => '*' }; - my $attrs = { %{$self->_resolved_attrs} }; - if (my $group_by = delete $attrs->{group_by}) { + + if (my $group_by = $attrs->{group_by}) { delete $attrs->{having}; + delete $attrs->{order_by}; my @distinct = (ref $group_by ? @$group_by : ($group_by)); # todo: try CONCAT for multi-column pk my @pk = $self->result_source->primary_columns; @@ -1161,14 +1161,15 @@ sub _count { # Separated out so pager can get the full count } } - $select = { count => { distinct => \@distinct } }; + $attrs->{select} = $group_by; + $attrs->{from} = [ { 'mesub' => (ref $self)->new($self->result_source, $attrs)->cursor->as_query } ]; } - $attrs->{select} = $select; + $attrs->{select} = { count => '*' }; $attrs->{as} = [qw/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/; + # offset, order by, group by, where and page are not needed to count. record_filter is cdbi + delete $attrs->{$_} for qw/rows offset order_by group_by where page pager record_filter/; my $tmp_rs = (ref $self)->new($self->result_source, $attrs); my ($count) = $tmp_rs->cursor->next; diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index 4bfa2e8..c27647c 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -95,6 +95,9 @@ sub _find_syntax { sub select { my ($self, $table, $fields, $where, $order, @rest) = @_; + local $self->{having_bind} = []; + local $self->{from_bind} = []; + if (ref $table eq 'SCALAR') { $table = $$table; } @@ -106,8 +109,7 @@ sub select { @rest = (-1) unless defined $rest[0]; die "LIMIT 0 Does Not Compute" if $rest[0] == 0; # and anyway, SQL::Abstract::Limit will cause a barf if we don't first - local $self->{having_bind} = []; - my ($sql, @ret) = $self->SUPER::select( + my ($sql, @where_bind) = $self->SUPER::select( $table, $self->_recurse_fields($fields), $where, $order, @rest ); $sql .= @@ -119,7 +121,7 @@ sub select { ) : '' ; - return wantarray ? ($sql, @ret, @{$self->{having_bind}}) : $sql; + return wantarray ? ($sql, @{$self->{from_bind}}, @where_bind, @{$self->{having_bind}}) : $sql; } sub insert { @@ -267,11 +269,10 @@ sub _recurse_from { } sub _bind_to_sql { - my $self = shift; - my $arr = shift; - my $sql = shift @$$arr; - $sql =~ s/\?/$self->_quote((shift @$$arr)->[1])/eg; - return $sql + my ($self, $arr) = @_; + my ($sql, @bind) = @{${$arr}}; + push (@{$self->{from_bind}}, @bind); + return $sql; } sub _make_as { diff --git a/t/count_distinct.t b/t/count_distinct.t new file mode 100644 index 0000000..b0ebd5f --- /dev/null +++ b/t/count_distinct.t @@ -0,0 +1,27 @@ +use strict; +use warnings; + +use Test::More; + +use lib qw(t/lib); + +use DBICTest; +use DBIC::SqlMakerTest; + +my $schema = DBICTest->init_schema(); + +eval "use DBD::SQLite"; +plan skip_all => 'needs DBD::SQLite for testing' if $@; +plan tests => 5; + +cmp_ok($schema->resultset("Tag")->count({ tag => 'Blue' }), + '==', 9, 'Count without DISTINCT ok'); + +cmp_ok($schema->resultset("Tag")->count({ tag => [ 'Blue', 'Shiny' ] }, { group_by => 'tag' }), + '==', 2, 'Count with single column group_by ok'); + +cmp_ok($schema->resultset("Tag")->count({ tag => 'Blue' }, { group_by => [ qw/tag cd/ ]}), + '==', 4, 'Count with multiple column group_by ok'); + +cmp_ok($schema->resultset("Tag")->count({ tag => 'Blue' }, { distinct => 1 }), + '==', 4, "Count with single column distinct ok"); diff --git a/t/from_subquery.t b/t/from_subquery.t new file mode 100644 index 0000000..3fd6cb0 --- /dev/null +++ b/t/from_subquery.t @@ -0,0 +1,186 @@ +use strict; +use warnings FATAL => 'all'; + +use Test::More; + +BEGIN { + eval "use SQL::Abstract 1.49"; + plan $@ + ? ( skip_all => "Needs SQLA 1.49+" ) + : ( tests => 8 ); +} + +use lib qw(t/lib); +use DBICTest; +use DBIC::SqlMakerTest; + +my $schema = DBICTest->init_schema(); +my $art_rs = $schema->resultset('Artist'); +my $cdrs = $schema->resultset('CD'); + +{ + my $cdrs2 = $cdrs->search({ + artist_id => { 'in' => $art_rs->search({}, { rows => 1 })->get_column( 'id' )->as_query }, + }); + + my $arr = $cdrs2->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT me.cdid,me.artist,me.title,me.year,me.genreid,me.single_track FROM cd me WHERE artist_id IN ( SELECT id FROM artist me LIMIT 1 )", + [], + ); +} + +{ + my $rs = $art_rs->search( + {}, + { + 'select' => [ + $cdrs->search({}, { rows => 1 })->get_column('id')->as_query, + ], + }, + ); + + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT (SELECT id FROM cd me LIMIT 1) FROM artist me", + [], + ); +} + +{ + my $rs = $art_rs->search( + {}, + { + '+select' => [ + $cdrs->search({}, { rows => 1 })->get_column('id')->as_query, + ], + }, + ); + + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT me.artistid, me.name, me.rank, me.charfield, (SELECT id FROM cd me LIMIT 1) FROM artist me", + [], + ); +} + +# simple from +{ + my $rs = $cdrs->search( + {}, + { + alias => 'cd2', + from => [ + { cd2 => $cdrs->search({ id => { '>' => 20 } })->as_query }, + ], + }, + ); + + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT cd2.cdid, cd2.artist, cd2.title, cd2.year, cd2.genreid, cd2.single_track FROM (SELECT me.cdid,me.artist,me.title,me.year,me.genreid,me.single_track FROM cd me WHERE ( id > ? ) ) cd2", + [ [ 'id', 20 ] ], + ); +} + +# nested from +{ + my $art_rs2 = $schema->resultset('Artist')->search({}, + { + from => [ { 'me' => 'artist' }, + [ { 'cds' => $cdrs->search({},{ 'select' => [\'me.artist as cds_artist' ]})->as_query }, + { 'me.artistid' => 'cds_artist' } ] ] + }); + + my $arr = $art_rs2->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT me.artistid, me.name, me.rank, me.charfield FROM artist me JOIN (SELECT me.artist as cds_artist FROM cd me) cds ON me.artistid = cds_artist", [] + ); + + +} + +# nested subquery in from +{ + my $rs = $cdrs->search( + {}, + { + alias => 'cd2', + from => [ + { cd2 => $cdrs->search( + { id => { '>' => 20 } }, + { + alias => 'cd3', + from => [ + { cd3 => $cdrs->search( { id => { '<' => 40 } } )->as_query } + ], + }, )->as_query }, + ], + }, + ); + + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT cd2.cdid, cd2.artist, cd2.title, cd2.year, cd2.genreid, cd2.single_track + FROM + (SELECT cd3.cdid,cd3.artist,cd3.title,cd3.year,cd3.genreid,cd3.single_track + FROM + (SELECT me.cdid,me.artist,me.title,me.year,me.genreid,me.single_track + FROM cd me WHERE ( id < ? ) ) cd3 + WHERE ( id > ? ) ) cd2", + [ [ 'id', 40 ], [ 'id', 20 ] ], + ); + +} + +{ + my $rs = $cdrs->search({ + year => { + '=' => $cdrs->search( + { artistid => { '=' => \'me.artistid' } }, + { alias => 'inner' } + )->get_column('year')->max_rs->as_query, + }, + }); + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track FROM cd me WHERE year = (SELECT MAX(inner.year) FROM cd inner WHERE artistid = me.artistid)", + [], + ); +} + +{ + my $rs = $cdrs->search( + {}, + { + alias => 'cd2', + from => [ + { cd2 => $cdrs->search({ title => 'Thriller' })->as_query }, + ], + }, + ); + + my $arr = $rs->as_query; + my ($query, @bind) = @{$$arr}; + is_same_sql_bind( + $query, \@bind, + "SELECT cd2.cdid, cd2.artist, cd2.title, cd2.year, cd2.genreid, cd2.single_track FROM (SELECT me.cdid,me.artist,me.title,me.year,me.genreid,me.single_track FROM cd me WHERE ( title = ? ) ) cd2", + [ [ 'title', 'Thriller' ] ], + ); +} + +__END__