From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 17 May 2009 23:10:22 +0000 (+0000)
Subject: Fixes for the diamond-relationship prefetch/join problem
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=1979278e1673ab611383389d9cc24c221bad6992;p=dbsrgits%2FDBIx-Class-Historic.git

Fixes for the diamond-relationship prefetch/join problem
The core of the issue was that resolve_prefetch calculated duplicate join alias numbers separate from resolve_join
In order to solve this, now the only join alias calculation happens in resolve_join (with prefetch being always merged as extra joins), and each join arrayref in from is labeled with the full relationship chain from me to the particular join. Then resolve_prefetch has to walk this chain and pull the necessary alias in order to generate the correct select
---

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index e21feeb..357bc51 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -2243,7 +2243,7 @@ sub related_resultset {
       "search_related: result source '" . $self->result_source->source_name .
         "' has no such relationship $rel")
       unless $rel_obj;
-    
+
     my ($from,$seen) = $self->_resolve_from($rel);
 
     my $join_count = $seen->{$rel};
@@ -2336,28 +2336,33 @@ sub current_source_alias {
   return ($self->{attrs} || {})->{alias} || 'me';
 }
 
+# This code is called by search_related, and makes sure there
+# is clear separation between the joins before, during, and
+# after the relationship. This information is needed later
+# 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 {
   my ($self, $extra_join) = @_;
   my $source = $self->result_source;
   my $attrs = $self->{attrs};
-  
+
   my $from = $attrs->{from}
     || [ { $attrs->{alias} => $source->from } ];
     
   my $seen = { %{$attrs->{seen_join}||{}} };
 
-  my $join = ($attrs->{join}
-               ? [ $attrs->{join}, $extra_join ]
-               : $extra_join);
-
   # we need to take the prefetch the attrs into account before we 
   # ->resolve_join as otherwise they get lost - captainL
-  my $merged = $self->_merge_attr( $join, $attrs->{prefetch} );
+  my $merged = $self->_merge_attr( $attrs->{join}, $attrs->{prefetch} );
+
+  push @$from, $source->resolve_join($merged, $attrs->{alias}, $seen) if ($merged);
+
+  ++$seen->{-relation_chain_depth};
+
+  push @$from, $source->resolve_join($extra_join, $attrs->{alias}, $seen);
 
-  $from = [
-    @$from,
-    ($join ? $source->resolve_join($merged, $attrs->{alias}, $seen) : ()),
-  ];
+  ++$seen->{-relation_chain_depth};
 
   return ($from,$seen);
 }
@@ -2479,12 +2484,12 @@ sub _resolved_attrs {
   if ( my $prefetch = delete $attrs->{prefetch} ) {
     $prefetch = $self->_merge_attr( {}, $prefetch );
     my @pre_order;
-    my $seen = { %{ $attrs->{seen_join} || {} } };
     foreach my $p ( ref $prefetch eq 'ARRAY' ? @$prefetch : ($prefetch) ) {
 
       # bring joins back to level of current class
+      my $join_map = $self->_joinpath_aliases ($attrs->{from}, $attrs->{seen_join});
       my @prefetch =
-        $source->resolve_prefetch( $p, $alias, $seen, \@pre_order, $collapse );
+        $source->resolve_prefetch( $p, $alias, $join_map, \@pre_order, $collapse );
       push( @{ $attrs->{select} }, map { $_->[0] } @prefetch );
       push( @{ $attrs->{as} },     map { $_->[1] } @prefetch );
     }
@@ -2500,6 +2505,25 @@ sub _resolved_attrs {
   return $self->{_attrs} = $attrs;
 }
 
+sub _joinpath_aliases {
+  my ($self, $fromspec, $seen) = @_;
+
+  my $paths = {};
+  return $paths unless ref $fromspec eq 'ARRAY';
+
+  for my $j (@$fromspec) {
+
+    next if ref $j ne 'ARRAY';
+    next if $j->[0]{-relation_chain_depth} < ( $seen->{-relation_chain_depth} || 0);
+
+    my $p = $paths;
+    $p = $p->{$_} ||= {} for @{$j->[0]{-join_path}};
+    push @{$p->{-join_aliases} }, $j->[0]{-join_alias};
+  }
+
+  return $paths;
+}
+
 sub _rollout_attr {
   my ($self, $attr) = @_;
   
diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index 9e67e66..f9e8654 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -1085,32 +1085,40 @@ Returns the join structure required for the related result source.
 =cut
 
 sub resolve_join {
-  my ($self, $join, $alias, $seen, $force_left) = @_;
-  $seen ||= {};
+  my ($self, $join, $alias, $seen, $force_left, $jpath) = @_;
+
+  # we need a supplied one, because we do in-place modifications, no returns
+  $self->throw_exception ('You must supply a seen hashref as the 3rd argument to resolve_join')
+    unless $seen;
+
   $force_left ||= { force => 0 };
+  $jpath ||= [];
+
   if (ref $join eq 'ARRAY') {
     return
       map {
         local $force_left->{force} = $force_left->{force};
-        $self->resolve_join($_, $alias, $seen, $force_left);
+        $self->resolve_join($_, $alias, $seen, $force_left, [@$jpath]);
       } @$join;
   } elsif (ref $join eq 'HASH') {
     return
       map {
-        my $as = ($seen->{$_} ? $_.'_'.($seen->{$_}+1) : $_);
+        my $as = ($seen->{$_} ? join ('_', $_, $seen->{$_} + 1) : $_);  # the actual seen value will be incremented below
         local $force_left->{force} = $force_left->{force};
         (
-          $self->resolve_join($_, $alias, $seen, $force_left),
+          $self->resolve_join($_, $alias, $seen, $force_left, [@$jpath]),
           $self->related_source($_)->resolve_join(
-            $join->{$_}, $as, $seen, $force_left
+            $join->{$_}, $as, $seen, $force_left, [@$jpath, $_]
           )
         );
       } keys %$join;
   } elsif (ref $join) {
     $self->throw_exception("No idea how to resolve join reftype ".ref $join);
   } else {
+
     my $count = ++$seen->{$join};
     my $as = ($count > 1 ? "${join}_${count}" : $join);
+
     my $rel_info = $self->relationship_info($join);
     $self->throw_exception("No such relationship ${join}") unless $rel_info;
     my $type;
@@ -1121,7 +1129,11 @@ sub resolve_join {
       $force_left->{force} = 1 if lc($type) eq 'left';
     }
     return [ { $as => $self->related_source($join)->from,
-               -join_type => $type },
+               -join_type => $type,
+               -join_path => [@$jpath, $join],
+               -join_alias => $as,
+               -relation_chain_depth => $seen->{-relation_chain_depth} || 0,
+             },
              $self->resolve_condition($rel_info->{cond}, $as, $alias) ];
   }
 }
@@ -1284,19 +1296,20 @@ in the supplied relationships. Examples:
 =cut
 
 sub resolve_prefetch {
-  my ($self, $pre, $alias, $seen, $order, $collapse) = @_;
-  $seen ||= {};
+  my ($self, $pre, $alias, $alias_map, $order, $collapse, $pref_path) = @_;
+  $pref_path ||= [];
+
   if( ref $pre eq 'ARRAY' ) {
     return
-      map { $self->resolve_prefetch( $_, $alias, $seen, $order, $collapse ) }
+      map { $self->resolve_prefetch( $_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ) }
         @$pre;
   }
   elsif( ref $pre eq 'HASH' ) {
     my @ret =
     map {
-      $self->resolve_prefetch($_, $alias, $seen, $order, $collapse),
+      $self->resolve_prefetch($_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ),
       $self->related_source($_)->resolve_prefetch(
-               $pre->{$_}, "${alias}.$_", $seen, $order, $collapse)
+               $pre->{$_}, "${alias}.$_", $alias_map, $order, $collapse, [ @$pref_path, $_] )
     } keys %$pre;
     return @ret;
   }
@@ -1305,8 +1318,17 @@ sub resolve_prefetch {
       "don't know how to resolve prefetch reftype ".ref($pre));
   }
   else {
-    my $count = ++$seen->{$pre};
-    my $as = ($count > 1 ? "${pre}_${count}" : $pre);
+
+    my $p = $alias_map;
+    $p = $p->{$_} for (@$pref_path, $pre);
+
+    $self->throw_exception (
+      "Unable to resolve prefetch $pre - join alias map does not contain an entry for path "
+      . join (' -> ', @$pref_path, $pre)
+    ) if (ref $p->{-join_aliases} ne 'ARRAY' or not @{$p->{-join_aliases}} );
+    
+    my $as = shift @{$p->{-join_aliases}};
+
     my $rel_info = $self->relationship_info( $pre );
     $self->throw_exception( $self->name . " has no such relationship '$pre'" )
       unless $rel_info;