From: Peter Rabbitson <ribasushi@cpan.org>
Date: Thu, 2 Jul 2009 13:52:35 +0000 (+0000)
Subject: Everything works, just need to fix join-path chaining over search_related (to guard... 
X-Git-Tag: v0.08108~21
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=f7f14dc88220135c2faf05ea6c4ea122690b52c7;p=dbsrgits%2FDBIx-Class.git

Everything works, just need to fix join-path chaining over search_related (to guard against obscure db quirks)
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 01a270f..64dfe60 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1322,54 +1322,38 @@ sub _switch_to_inner_join_if_needed {
   # the target as it is not in the parseable part of {from}
   return $from if @$from == 1;
 
-  my (@switch_idx, $found_target);
-
+  my $switch_branch;
   JOINSCAN:
-  for my $i (1 .. $#$from) {
-
-    push @switch_idx, $i;
-    my $j = $from->[$i];
-    my $jalias = $j->[0]{-alias};
-
-    # we found our current target - delete any siblings (same level joins)
-    # and bail out
-    if ($jalias eq $alias) {
-      $found_target++;
-
-      my $cur_depth = $j->[0]{-relation_chain_depth};
-      # we are -1, so look at -2
-      while (@switch_idx > 1
-              && $from->[$switch_idx[-2]][0]{-relation_chain_depth} == $cur_depth
-      ) {
-        splice @switch_idx, -2, 1;
-      }
-
+  for my $j (@{$from}[1 .. $#$from]) {
+    if ($j->[0]{-alias} eq $alias) {
+      $switch_branch = $j->[0]{-join_path};
       last JOINSCAN;
     }
   }
 
   # something else went wrong
-  return $from unless $found_target;
+  return $from unless $switch_branch;
 
   # So it looks like we will have to switch some stuff around.
   # local() is useless here as we will be leaving the scope
   # anyway, and deep cloning is just too fucking expensive
   # So replace the inner hashref manually
-  my @new_from;
-  my $sw_idx = { map { $_ => 1 } @switch_idx };
+  my @new_from = ($from->[0]);
+  my $sw_idx = { map { $_ => 1 } @$switch_branch };
 
-  for my $i (0 .. $#$from) {
-    if ($sw_idx->{$i}) {
-      my %attrs = %{$from->[$i][0]};
-      delete $attrs{-join_type};
+  for my $j (@{$from}[1 .. $#$from]) {
+    my $jalias = $j->[0]{-alias};
 
+    if ($sw_idx->{$jalias}) {
+      my %attrs = %{$j->[0]};
+      delete $attrs{-join_type};
       push @new_from, [
         \%attrs,
-        @{$from->[$i]}[ 1 .. $#{$from->[$i]} ],
+        @{$j}[ 1 .. $#$j ],
       ];
     }
     else {
-      push @new_from, $from->[$i];
+      push @new_from, $j;
     }
   }
 
@@ -2704,6 +2688,7 @@ sub _chain_relationship {
   # 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]) {
@@ -2712,6 +2697,17 @@ sub _chain_relationship {
       last;
     }
   }
+
+# alternative way to scan the entire chain - not backwards compatible
+#  for my $j (reverse @$from) {
+#    next unless ref $j eq 'ARRAY';
+#    if ($j->[0]{-join_path} && $j->[0]{-join_path}[-1] eq $rel) {
+#      $j->[0]{-relation_chain_depth}++;
+#      $already_joined++;
+#      last;
+#    }
+#  }
+
   unless ($already_joined) {
     push @$from, $source->_resolve_join($rel, $attrs->{alias}, $seen);
   }
diff --git a/t/prefetch/count.t b/t/prefetch/count.t
index 30aed7e..49370a4 100644
--- a/t/prefetch/count.t
+++ b/t/prefetch/count.t
@@ -4,8 +4,9 @@ use warnings;
 use Test::More;
 use lib qw(t/lib);
 use DBICTest;
+use DBIC::SqlMakerTest;
 
-plan tests => 11;
+plan tests => 23;
 
 my $schema = DBICTest->init_schema();
 
@@ -40,3 +41,61 @@ $artist->create_related ('cds', { title => 'yyy', year => '1999' });
 is($artist_rs->related_resultset('cds')->count, 1, "1 CDs counted on a brand new artist");
 is(scalar($artist_rs->related_resultset('cds')->all), 1, "1 CDs prefetched on a brand new artist (count == fetch)");
 
+# Really fuck shit up with one more cd and some insanity
+# this doesn't quite work as there are the prefetch gets lost
+# on search_related. This however is too esoteric to fix right
+# now
+
+my $cd2 = $artist->create_related ('cds', {
+    title => 'zzz',
+    year => '1999',
+    tracks => [{ title => 'ping' }, { title => 'pong' }],
+});
+
+my $cds = $cd2->search_related ('artist', {}, { join => 'twokeys' })
+                  ->search_related ('cds');
+my $tracks = $cds->search_related ('tracks');
+
+is($tracks->count, 2, "2 Tracks counted on cd via artist via one of the cds");
+is(scalar($tracks->all), 2, "2 Track objects on cd via artist via one of the cds");
+
+is($cds->count, 2, "2 CDs counted on artist via one of the cds");
+is(scalar($cds->all), 2, "2 CD objectson artist via one of the cds");
+
+# make sure the join collapses all the way
+is_same_sql_bind (
+  $tracks->count_rs->as_query,
+  '(
+    SELECT COUNT( * )
+      FROM artist me
+      LEFT JOIN twokeys twokeys ON twokeys.artist = me.artistid
+      JOIN cd cds ON cds.artist = me.artistid
+      JOIN track tracks ON tracks.cd = cds.cdid
+    WHERE ( me.artistid = ? )
+  )',
+  [ [ 'me.artistid' => 4 ] ],
+);
+
+
+TODO: {
+  local $TODO = "Chaining with prefetch is fundamentally broken";
+
+  my $queries;
+  $schema->storage->debugcb ( sub { $queries++ } );
+  $schema->storage->debug (1);
+
+  my $cds = $cd2->search_related ('artist', {}, { prefetch => { cds => 'tracks' }, join => 'twokeys' })
+                  ->search_related ('cds');
+
+  my $tracks = $cds->search_related ('tracks');
+
+  is($tracks->count, 2, "2 Tracks counted on cd via artist via one of the cds");
+  is(scalar($tracks->all), 2, "2 Tracks prefetched on cd via artist via one of the cds");
+  is($tracks->count, 2, "Cached 2 Tracks counted on cd via artist via one of the cds");
+
+  is($cds->count, 2, "2 CDs counted on artist via one of the cds");
+  is(scalar($cds->all), 2, "2 CDs prefetched on artist via one of the cds");
+  is($cds->count, 2, "Cached 2 CDs counted on artist via one of the cds");
+
+  is ($queries, 3, '2 counts + 1 prefetch?');
+}