Skip to content

Commit

Permalink
Warn when distinct is used with group_by
Browse files Browse the repository at this point in the history
  • Loading branch information
ribasushi committed Sep 15, 2009
1 parent 1a62530 commit 00f3b1c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
10 changes: 8 additions & 2 deletions lib/DBIx/Class/ResultSet.pm
Expand Up @@ -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} ||= {};
Expand Down Expand Up @@ -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
Expand Down
20 changes: 14 additions & 6 deletions t/60core.t
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');

Expand All @@ -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(
{},
Expand Down

0 comments on commit 00f3b1c

Please sign in to comment.