From: Peter Rabbitson Date: Fri, 6 Aug 2010 23:09:23 +0000 (+0200) Subject: Fix a corner case when a distinct plus an unqualified order_by on one X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=d955e93801fcdb5f019866252d9155d727aad646;p=dbsrgits%2FDBIx-Class-Historic.git Fix a corner case when a distinct plus an unqualified order_by on one of the distinct-ed columns resulted in double-specification in the GROUP BY clause --- diff --git a/Changes b/Changes index 72615de..83fa3cf 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,8 @@ Revision history for DBIx::Class display properly in DBIC_TRACE - Incomplete exception thrown on relationship auto-fk-inference failures + - Fixed distinct with order_by to not double-specify the same + column in the GROUP BY clause * Misc - Makefile.PL no longer imports GetOptions() to interoperate diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 004368a..2fa0717 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -2974,21 +2974,33 @@ sub _resolved_attrs { carp ("Useless use of distinct on a grouped resultset ('distinct' is ignored when a 'group_by' is present)"); } else { - $attrs->{group_by} = [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ]; + my $storage = $self->result_source->schema->storage; + my $rs_column_list = $storage->_resolve_column_info ($attrs->{from}); + + my $group_spec = $attrs->{group_by} = []; + my %group_index; + for (@{$attrs->{select}}) { + if (! ref($_) or ref ($_) ne 'HASH' ) { + push @$group_spec, $_; + $group_index{$_}++; + if ($rs_column_list->{$_} and $_ !~ /\./ ) { + # add a fully qualified version as well + $group_index{"$rs_column_list->{$_}{-source_alias}.$_"}++; + } + } + } # add any order_by parts that are not already present in the group_by # we need to be careful not to add any named functions/aggregates # i.e. select => [ ... { count => 'foo', -as 'foocount' } ... ] - my %already_grouped = map { $_ => 1 } (@{$attrs->{group_by}}); - - my $storage = $self->result_source->schema->storage; + for my $chunk ($storage->_parse_order_by($attrs->{order_by})) { - my $rs_column_list = $storage->_resolve_column_info ($attrs->{from}); + # only consider real columns (for functions the user got to do an explicit group_by) + my $colinfo = $rs_column_list->{$chunk} + or next; - for my $chunk ($storage->_parse_order_by($attrs->{order_by})) { - if ($rs_column_list->{$chunk} && not $already_grouped{$chunk}++) { - push @{$attrs->{group_by}}, $chunk; - } + $chunk = "$colinfo->{-source_alias}.$chunk" if $chunk !~ /\./; + push @$group_spec, $chunk unless $group_index{$chunk}++; } } } diff --git a/t/prefetch/grouped.t b/t/prefetch/grouped.t index d2d1f17..8c84462 100644 --- a/t/prefetch/grouped.t +++ b/t/prefetch/grouped.t @@ -219,7 +219,7 @@ for ($cd_rs->all) { FROM ( SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track FROM cd me - GROUP BY me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track, cdid + GROUP BY me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track ORDER BY cdid ) me LEFT JOIN tags tags ON tags.cd = me.cdid diff --git a/t/search/distinct.t b/t/search/distinct.t new file mode 100644 index 0000000..5ec213a --- /dev/null +++ b/t/search/distinct.t @@ -0,0 +1,32 @@ +use strict; +use warnings; + +use Test::More; +use Test::Exception; + +use lib qw(t/lib); +use DBIC::SqlMakerTest; +use DBICTest; + +my $schema = DBICTest->init_schema(); + +# make sure order + distinct do not double-inject group criteria +my $year_rs = $schema->resultset ('CD')->search ({}, { + distinct => 1, + columns => [qw/year/], + order_by => 'year', +}); + +is_same_sql_bind ( + $year_rs->as_query, + '( + SELECT me.year + FROM cd me + GROUP BY me.year + ORDER BY year + )', + [], + 'Correct GROUP BY', +); + +done_testing;