From: Peter Rabbitson Date: Sun, 12 Dec 2010 07:10:54 +0000 (+0100) Subject: Back out "support for prefetch from resultsource using extended_rels" X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=abf8d91e24dae052a0af4b65ffee4e72044d54bb;p=dbsrgits%2FDBIx-Class-Historic.git Back out "support for prefetch from resultsource using extended_rels" Blindly taking the condition retuned by an incomplete (not all pieces of the hashref are supplied) call to the condition maker, and attempting to order by its keys assuming it is a hash is... naive --- diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index 780aaa9..4e9408a 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -1548,7 +1548,7 @@ sub resolve_condition { # Resolves the passed condition to a concrete query fragment. If given an alias, # returns a join condition; if given an object, inverts that object to produce # a related conditional from that object. -our $UNRESOLVABLE_CONDITION = \'1 = 0'; +our $UNRESOLVABLE_CONDITION = \ '1 = 0'; sub _resolve_condition { my ($self, $cond, $as, $for, $rel) = @_; @@ -1675,7 +1675,7 @@ sub _resolve_prefetch { if ($rel_info->{attrs}{accessor} && $rel_info->{attrs}{accessor} eq 'multi') { $self->throw_exception( "Can't prefetch has_many ${pre} (join cond too complex)") - unless (ref($rel_info->{cond}) eq 'HASH' || ref($rel_info->{cond}) eq 'CODE'); + unless ref($rel_info->{cond}) eq 'HASH'; my $dots = @{[$as_prefix =~ m/\./g]} + 1; # +1 to match the ".${as_prefix}" if (my ($fail) = grep { @{[$_ =~ m/\./g]} == $dots } @@ -1697,17 +1697,9 @@ sub _resolve_prefetch { $collapse->{".${as_prefix}${pre}"} = [ $rel_source->_pri_cols ]; # action at a distance. prepending the '.' allows simpler code # in ResultSet->_collapse_result - - if (ref $rel_info->{cond} eq 'HASH') { - my @key = map { (/^foreign\.(.+)$/ ? ($1) : ()); } - keys %{$rel_info->{cond}}; - push @$order, map { "${as}.$_" } @key; - } else { # ref $rel_info->{cond} eq 'CODE' - # call the cond to get the keys... - my $cond_data = $rel_info->{cond}->({ foreign_alias => $as, - self_alias => $alias }); - push @$order, keys %$cond_data; - } + my @key = map { (/^foreign\.(.+)$/ ? ($1) : ()); } + keys %{$rel_info->{cond}}; + push @$order, map { "${as}.$_" } @key; if (my $rel_order = $rel_info->{attrs}{order_by}) { # this is kludgy and incomplete, I am well aware diff --git a/t/lib/DBICTest/Schema/Artist.pm b/t/lib/DBICTest/Schema/Artist.pm index 48538bc..e86aae0 100644 --- a/t/lib/DBICTest/Schema/Artist.pm +++ b/t/lib/DBICTest/Schema/Artist.pm @@ -63,13 +63,18 @@ __PACKAGE__->has_many( }, ); + __PACKAGE__->has_many( - cds_90s => 'DBICTest::Schema::CD', + cds_84 => 'DBICTest::Schema::CD', sub { my $args = shift; return ( { "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, - "$args->{foreign_alias}.year" => { '>' => 1989, '<' => 2000 }, + "$args->{foreign_alias}.year" => 1984, + }, + $args->{self_rowobj} && { + "$args->{foreign_alias}.artist" => $args->{self_rowobj}->artistid, + "$args->{foreign_alias}.year" => 1984, } ); } @@ -77,16 +82,12 @@ __PACKAGE__->has_many( __PACKAGE__->has_many( - cds_84 => 'DBICTest::Schema::CD', + cds_90s => 'DBICTest::Schema::CD', sub { my $args = shift; return ( { "$args->{foreign_alias}.artist" => { -ident => "$args->{self_alias}.artistid" }, - "$args->{foreign_alias}.year" => 1984, - }, - $args->{self_rowobj} && { - "$args->{foreign_alias}.artist" => $args->{self_rowobj}->artistid, - "$args->{foreign_alias}.year" => 1984, + "$args->{foreign_alias}.year" => { '>' => 1989, '<' => 2000 }, } ); } diff --git a/t/relationship/custom.t b/t/relationship/custom.t index 9809a65..4fb96ff 100644 --- a/t/relationship/custom.t +++ b/t/relationship/custom.t @@ -9,10 +9,13 @@ use DBIC::SqlMakerTest; my $schema = DBICTest->init_schema(); -my $artist = $schema->resultset("Artist")->create({ name => 'Michael Jackson', rank => 20 }); -my $artist2 = $schema->resultset("Artist")->create({ name => 'Chico Buarque', rank => 1 }) ; -my $artist3 = $schema->resultset("Artist")->create({ name => 'Ziraldo', rank => 1 }); -my $artist4 = $schema->resultset("Artist")->create({ name => 'Paulo Caruso', rank => 20 }); +$schema->resultset('Artist')->delete; +$schema->resultset('CD')->delete; + +my $artist = $schema->resultset("Artist")->create({ artistid => 21, name => 'Michael Jackson', rank => 20 }); +my $artist2 = $schema->resultset("Artist")->create({ artistid => 22, name => 'Chico Buarque', rank => 1 }) ; +my $artist3 = $schema->resultset("Artist")->create({ artistid => 23, name => 'Ziraldo', rank => 1 }); +my $artist4 = $schema->resultset("Artist")->create({ artistid => 24, name => 'Paulo Caruso', rank => 20 }); my @artworks; @@ -30,6 +33,7 @@ foreach my $artwork (@artworks) { $artwork->create_related('artwork_to_artist', { artist => $_ }) for ($artist3, $artist4); } + my $cds_80s_rs = $artist->cds_80s; is_same_sql_bind($cds_80s_rs->as_query, '(SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track FROM cd me'. @@ -77,21 +81,6 @@ is(@cds_90s_95, 1, '1 90s (95) cds found even with non-optimized search'); map { ok($_->year == 1995) } @cds_90s_95; # search for all artists prefetching published cds in the 80s... -##### -# the join must be a prefetch, but it can't work until the collapse rewrite is finished -# (right-side vs left-side order) -##### -lives_ok { - my @all_artists_with_80_cds = $schema->resultset("Artist")->search - ({ 'cds_80s.cdid' => { '!=' => undef } }, { prefetch => 'cds_80s' })->all; - - is_deeply - ([ sort ( map { $_->year } map { $_->cds_80s->all } @all_artists_with_80_cds ) ], - [ sort (1980..1989, 1980..1985) ], - '16 correct cds found' - ); -} 'prefetchy-fetchy-fetch'; - my @all_artists_with_80_cds = $schema->resultset("Artist")->search ({ 'cds_80s.cdid' => { '!=' => undef } }, { join => 'cds_80s', distinct => 1 }); @@ -101,6 +90,24 @@ is_deeply( '16 correct cds found' ); +TODO: { +local $TODO = 'Prefetch on custom rels can not work until the collapse rewrite is finished ' + . '(currently collapser requires a right-side (which is indeterministic) order-by)'; +lives_ok { + +my @all_artists_with_80_cds_pref = $schema->resultset("Artist")->search + ({ 'cds_80s.cdid' => { '!=' => undef } }, { prefetch => 'cds_80s' }); + +is_deeply( + [ sort ( map { $_->year } map { $_->cds_80s->all } @all_artists_with_80_cds_pref ) ], + [ sort (1980..1989, 1980..1985) ], + '16 correct cds found' +); + +} 'prefetchy-fetchy-fetch'; +} # end of TODO + + # try to create_related a 80s cd throws_ok { $artist->create_related('cds_80s', { title => 'related creation 1' });