Merge 'search_related_prefetch' into 'trunk'

r6815@Thesaurus (orig r6814):  ribasushi | 2009-06-28 10:32:42 +0200
Branch to explore double joins on search_related
r6816@Thesaurus (orig r6815):  ribasushi | 2009-06-28 10:34:16 +0200
Thetest case that started it all
r6817@Thesaurus (orig r6816):  ribasushi | 2009-06-28 10:35:11 +0200
The proposed fix (do not add an extra join if it is already present in the topmost join)
r6818@Thesaurus (orig r6817):  ribasushi | 2009-06-28 11:04:26 +0200
Minor omission
r6819@Thesaurus (orig r6818):  ribasushi | 2009-06-28 11:07:33 +0200
Adjust a couple of tests for new behavior (thus all of this might be backwards incompatible to the point of being useless):
The counts in t/90join_torture.t are now 5*3, not 5*3*3, as a second join is not induced by search_related
The raw sql scan in t/prefetch/standard.t is just silly, won't even try to understand it
Just to maintain the TreeLike folding, I add a 3rd children join which was inserted by search_related before the code changes

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index b012e03..d1db895 100644 (file)
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -2450,7 +2450,7 @@ sub related_resultset {
         "' has no such relationship $rel")
       unless $rel_info;
 
-    my ($from,$seen) = $self->_resolve_from($rel);
+    my ($from,$seen) = $self->_chain_relationship($rel);
 
     my $join_count = $seen->{$rel};
     my $alias = ($join_count > 1 ? join('_', $rel, $join_count) : $rel);
@@ -2548,7 +2548,7 @@ sub current_source_alias {
 # in order to properly resolve prefetch aliases (any alias
 # with a relation_chain_depth less than the depth of the
 # current prefetch is not considered)
-sub _resolve_from {
+sub _chain_relationship {
   my ($self, $rel) = @_;
   my $source = $self->result_source;
   my $attrs = $self->{attrs};
@@ -2569,11 +2569,29 @@ sub _resolve_from {
   # ->_resolve_join as otherwise they get lost - captainL
   my $merged = $self->_merge_attr( $attrs->{join}, $attrs->{prefetch} );
 
-  push @$from, $source->_resolve_join($merged, $attrs->{alias}, $seen) if ($merged);
+  my @requested_joins = $source->_resolve_join($merged, $attrs->{alias}, $seen);
+
+  push @$from, @requested_joins;
 
   ++$seen->{-relation_chain_depth};
 
-  push @$from, $source->_resolve_join($rel, $attrs->{alias}, $seen);
+  # if $self already had a join/prefetch specified on it, the requested
+  # $rel might very well be already included. What we do in this case
+  # is effectively a no-op (except that we bump up the chain_depth on
+  # the join in question so we could tell it *is* the search_related)
+  my $already_joined;
+
+  # we consider the last one thus reverse
+  for my $j (reverse @requested_joins) {
+    if ($rel eq $j->[0]{-join_path}[-1]) {
+      $j->[0]{-relation_chain_depth}++;
+      $already_joined++;
+      last;
+    }
+  }
+  unless ($already_joined) {
+    push @$from, $source->_resolve_join($rel, $attrs->{alias}, $seen);
+  }
 
   ++$seen->{-relation_chain_depth};
 

diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index d6fd004..4eca0f8 100644 (file)
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -893,7 +893,7 @@ sub add_relationship {
   }
   return unless $f_source; # Can't test rel without f_source
 
-  eval { $self->_resolve_join($rel, 'me') };
+  eval { $self->_resolve_join($rel, 'me', {}) };
 
   if ($@) { # If the resolve failed, back out and re-throw the error
     delete $rels{$rel}; #
@@ -1117,6 +1117,8 @@ sub _resolve_join {
     $self->throw_exception("No idea how to resolve join reftype ".ref $join);
   } else {
 
+    return() unless defined $join;
+
     my $count = ++$seen->{$join};
     my $as = ($count > 1 ? "${join}_${count}" : $join);
 

diff --git a/t/90join_torture.t b/t/90join_torture.t
index 0339bfc..6eeda5a 100644 (file)
--- a/t/90join_torture.t
+++ b/t/90join_torture.t
@@ -46,9 +46,9 @@ cmp_ok(scalar @cds, '==', 1, "condition based on inherited join okay");
 
 my $rs3 = $rs2->search_related('cds');
 
-cmp_ok(scalar($rs3->all), '==', 45, "All cds for artist returned");
+cmp_ok(scalar($rs3->all), '==', 15, "All cds for artist returned");
 
-cmp_ok($rs3->count, '==', 45, "All cds for artist returned via count");
+cmp_ok($rs3->count, '==', 15, "All cds for artist returned via count");
 
 my $rs4 = $schema->resultset("CD")->search({ 'artist.artistid' => '1' }, { join => ['tracks', 'artist'], prefetch => 'artist' });
 my @rs4_results = $rs4->all;

diff --git a/t/prefetch/count.t b/t/prefetch/count.t
new file mode 100644 (file)
index 0000000..fac478c
--- /dev/null
+++ b/t/prefetch/count.t
@@ -0,0 +1,24 @@
+use strict;
+use warnings;
+
+use Test::More;
+use lib qw(t/lib);
+use DBICTest;
+
+plan tests => 5;
+
+my $schema = DBICTest->init_schema();
+
+my $cd_rs = $schema->resultset('CD')->search (
+  { 'tracks.cd' => { '!=', undef } },
+  { prefetch => ['tracks', 'artist'] },
+);
+
+
+is($cd_rs->count, 5, 'CDs with tracks count');
+is($cd_rs->search_related('tracks')->count, 15, 'Tracks associated with CDs count (before SELECT()ing)');
+
+is($cd_rs->all, 5, 'Amount of CD objects with tracks');
+is($cd_rs->search_related('tracks')->count, 15, 'Tracks associated with CDs count (after SELECT()ing)');
+
+is($cd_rs->search_related ('tracks')->all, 15, 'Track objects associated with CDs (after SELECT()ing)');

diff --git a/t/prefetch/standard.t b/t/prefetch/standard.t
index 56cf77d..9fe2ca4 100644 (file)
--- a/t/prefetch/standard.t
+++ b/t/prefetch/standard.t
@@ -1,5 +1,5 @@
 use strict;
-use warnings;  
+use warnings;
 
 use Test::More;
 use Test::Exception;
@@ -13,12 +13,7 @@ my $orig_debug = $schema->storage->debug;
 
 use IO::File;
 
-BEGIN {
-    eval "use DBD::SQLite";
-    plan $@
-        ? ( skip_all => 'needs DBD::SQLite for testing' )
-        : ( tests => 45 );
-}
+plan tests => 44;
 
 my $queries = 0;
 $schema->storage->debugcb(sub { $queries++; });
@@ -227,29 +222,11 @@ is(eval { $tree_like->children->first->children->first->name }, 'quux',
 
 $tree_like = eval { $schema->resultset('TreeLike')->search(
     { 'children.id' => 3, 'children_2.id' => 6 }, 
-    { join => [qw/children children/] }
+    { join => [qw/children children children/] }
   )->search_related('children', { 'children_4.id' => 7 }, { prefetch => 'children' }
   )->first->children->first; };
 is(eval { $tree_like->name }, 'fong', 'Tree with multiple has_many joins ok');
 
-# test that collapsed joins don't get a _2 appended to the alias
-
-my $sql = '';
-$schema->storage->debugcb(sub { $sql = $_[1] });
-$schema->storage->debug(1);
-
-eval {
-  my $row = $schema->resultset('Artist')->search_related('cds', undef, {
-    join => 'tracks',
-    prefetch => 'tracks',
-  })->search_related('tracks')->first;
-};
-
-like( $sql, qr/^SELECT tracks_2\.trackid/, "join not collapsed for search_related" );
-
-$schema->storage->debug($orig_debug);
-$schema->storage->debugobj->callback(undef);
-
 $rs = $schema->resultset('Artist');
 $rs->create({ artistid => 4, name => 'Unknown singer-songwriter' });
 $rs->create({ artistid => 5, name => 'Emo 4ever' });