From: Peter Rabbitson Date: Tue, 6 Apr 2010 03:36:04 +0000 (+0000) Subject: Fix embarassing join optimizer bug X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=faeb2407c49717d38874b21749659ac2c632bad7;p=dbsrgits%2FDBIx-Class-Historic.git Fix embarassing join optimizer bug --- diff --git a/Changes b/Changes index 9b4f636..d8659c5 100644 --- a/Changes +++ b/Changes @@ -17,6 +17,8 @@ Revision history for DBIx::Class is unresolvable - Fix the join-optimiser to consider unqualified column names whenever possible + - Fix an issue with multiple same-table joins confusing the join + optimizier - Add has_relationship method to row objects - Fix regression in set_column on PK-less objects - Better error text on malformed/missing relationships diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm index 2d10e6c..32e0ea3 100644 --- a/lib/DBIx/Class/Storage/DBIHacks.pm +++ b/lib/DBIx/Class/Storage/DBIHacks.pm @@ -273,7 +273,6 @@ sub _resolve_aliastypes_from_select_args { $aliases_by_type->{$type}{$alias} = 1 if ($piece =~ $al_re); } } - } # now loop through unqualified column names, and try to locate them within @@ -308,7 +307,7 @@ sub _resolve_aliastypes_from_select_args { for my $type (keys %$aliases_by_type) { for my $alias (keys %{$aliases_by_type->{$type}}) { $aliases_by_type->{$type}{$_} = 1 - for (map { keys %$_ } @{ $alias_list->{$alias}{-join_path} || [] }); + for (map { values %$_ } @{ $alias_list->{$alias}{-join_path} || [] }); } } @@ -453,7 +452,7 @@ sub _straight_join_to_node { # anyway, and deep cloning is just too fucking expensive # So replace the first hashref in the node arrayref manually my @new_from = ($from->[0]); - my $sw_idx = { map { values %$_ => 1 } @$switch_branch }; + my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path for my $j (@{$from}[1 .. $#$from]) { my $jalias = $j->[0]{-alias}; diff --git a/t/90join_torture.t b/t/90join_torture.t index 6eeda5a..cd7d2ef 100644 --- a/t/90join_torture.t +++ b/t/90join_torture.t @@ -1,13 +1,13 @@ use strict; -use warnings; +use warnings; use Test::More; +use Test::Exception; use lib qw(t/lib); use DBICTest; +use DBIC::SqlMakerTest; my $schema = DBICTest->init_schema(); -plan tests => 22; - { my $rs = $schema->resultset( 'CD' )->search( { @@ -25,11 +25,10 @@ plan tests => 22; ], } ); - - eval { + + lives_ok { my @rows = $rs->all(); }; - is( $@, '' ); } @@ -106,7 +105,7 @@ my $merge_rs_2 = $schema->resultset("Artist")->search({ }, { join => 'cds' })->s is(scalar(@{$merge_rs_2->{attrs}->{join}}), 1, 'only one join kept when inherited'); my $merge_rs_2_cd = $merge_rs_2->next; -eval { +lives_ok (sub { my @rs_with_prefetch = $schema->resultset('TreeLike') ->search( @@ -115,9 +114,7 @@ eval { prefetch => [ 'parent', { 'children' => 'parent' } ], }); -}; - -ok(!$@, "pathological prefetch ok"); +}, 'pathological prefetch ok'); my $rs = $schema->resultset("Artist")->search({}, { join => 'twokeys' }); my $second_search_rs = $rs->search({ 'cds_2.cdid' => '2' }, { join => @@ -125,4 +122,36 @@ my $second_search_rs = $rs->search({ 'cds_2.cdid' => '2' }, { join => is(scalar(@{$second_search_rs->{attrs}->{join}}), 3, 'both joins kept'); ok($second_search_rs->next, 'query on double joined rel runs okay'); -1; +# test joinmap pruner +lives_ok ( sub { + my $rs = $schema->resultset('Artwork')->search ( + { + }, + { + distinct => 1, + join => [ + { artwork_to_artist => 'artist' }, + { cd => 'artist' }, + ], + }, + ); + + is_same_sql_bind ( + $rs->count_rs->as_query, + '( + SELECT COUNT( * ) + FROM ( + SELECT me.cd_id + FROM cd_artwork me + JOIN cd cd ON cd.cdid = me.cd_id + JOIN artist artist_2 ON artist_2.artistid = cd.artist + GROUP BY me.cd_id + ) count_subq + )', + [], + ); + + ok (defined $rs->count); +}); + +done_testing;