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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
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;