From: Peter Rabbitson Date: Tue, 15 Sep 2009 07:43:46 +0000 (+0000) Subject: Warn when distinct is used with group_by X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=00f3b1c7f44dd29244b4ecdf99150278700ee14f;p=dbsrgits%2FDBIx-Class-Historic.git Warn when distinct is used with group_by --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index d528023..005b605 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -2896,7 +2896,12 @@ sub _resolved_attrs { # generate the distinct induced group_by early, as prefetch will be carried via a # subquery (since a group_by is present) if (delete $attrs->{distinct}) { - $attrs->{group_by} ||= [ grep { !ref($_) || (ref($_) ne 'HASH') } @{$attrs->{select}} ]; + if ($attrs->{group_by}) { + 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}} ]; + } } $attrs->{collapse} ||= {}; @@ -3515,7 +3520,8 @@ done. =back -Set to 1 to group by all columns. +Set to 1 to group by all columns. If the resultset already has a group_by +attribute, this setting is ignored and an appropriate warning is issued. =head2 where diff --git a/t/60core.t b/t/60core.t index 6094c39..b62b82d 100644 --- a/t/60core.t +++ b/t/60core.t @@ -3,6 +3,7 @@ use warnings; use Test::More; use Test::Exception; +use Test::Warn; use lib qw(t/lib); use DBICTest; use DBIC::SqlMakerTest; @@ -35,10 +36,10 @@ ok($art->update, 'Update run'); my %not_dirty = $art->get_dirty_columns(); is(scalar(keys(%not_dirty)), 0, 'Nothing is dirty'); -eval { +throws_ok ( sub { my $ret = $art->make_column_dirty('name2'); -}; -ok(defined($@), 'Failed to make non-existent column dirty'); +}, qr/No such column 'name2'/, 'Failed to make non-existent column dirty'); + $art->make_column_dirty('name'); my %fake_dirty = $art->get_dirty_columns(); is(scalar(keys(%fake_dirty)), 1, '1 fake dirty column'); @@ -221,9 +222,9 @@ SKIP: { isa_ok($tdata{'last_updated_on'}, 'DateTime', 'inflated accessored column'); } -eval { $schema->class("Track")->load_components('DoesNotExist'); }; - -ok $@, $@; +throws_ok (sub { + $schema->class("Track")->load_components('DoesNotExist'); +}, qr!Can't locate DBIx/Class/DoesNotExist.pm!, 'exception on nonexisting component'); is($schema->class("Artist")->field_name_for->{name}, 'artist name', 'mk_classdata usage ok'); @@ -238,6 +239,13 @@ my $collapsed_or_rs = $or_rs->search ({}, { distinct => 1 }); # induce collapse is ($collapsed_or_rs->all, 4, 'Collapsed joined search with OR returned correct number of rows'); is ($collapsed_or_rs->count, 4, 'Collapsed search count with OR ok'); +# make sure sure distinct on a grouped rs is warned about +my $cd_rs = $schema->resultset ('CD') + ->search ({}, { distinct => 1, group_by => 'title' }); +warnings_exist (sub { + $cd_rs->next; +}, qr/Useless use of distinct/, 'UUoD warning'); + { my $tcount = $schema->resultset('Track')->search( {},