Fix stupid optimizer bug with has_many joined over might_have

diff --git a/Changes b/Changes
index c5220ec..4b1567a 100644 (file)
--- a/Changes
+++ b/Changes
@@ -60,6 +60,8 @@ Revision history for DBIx::Class
           sub-selects in the selectors list (correlated subqueries)
         - Fix inconsistency between $rs->next with and without HRI when all
           the "root" columns are in fact injected from the right rs side
+        - Fix the join optimizer to correctly preserve the non-multi path to
+          a multi relationship ( x -> might_have y -> has_many z )
 
     * Misc
         - Rewire all warnings to a new Carp-like implementation internal

diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 10dd178..583045c 100644 (file)
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -39,7 +39,14 @@ sub _prune_unused_joins {
 
   my @newfrom = $from->[0]; # FROM head is always present
 
-  my %need_joins = (map { %{$_||{}} } (values %$aliastypes) );
+  my %need_joins;
+  for (values %$aliastypes) {
+    # add all requested aliases
+    $need_joins{$_} = 1 for keys %$_;
+
+    # add all their parents (as per joinpath which is an AoH { table => alias })
+    $need_joins{$_} = 1 for map { values %$_ } map { @$_ } values %$_;
+  }
   for my $j (@{$from}[1..$#$from]) {
     push @newfrom, $j if (
       (! $j->[0]{-alias}) # legacy crap
@@ -119,8 +126,8 @@ sub _adjust_select_args_for_complex_prefetch {
     my $inner_aliastypes =
       $self->_resolve_aliastypes_from_select_args( $inner_from, $inner_select, $where, $inner_attrs );
 
-    # if a multi-type non-selecting (only restricting) join was needed in the subquery
-    # add a group_by to simulate the collapse in the subq
+    # we need to simulate collapse in the subq if a multiplying join is pulled
+    # by being a non-selecting restrictor
     if (
       ! $inner_attrs->{group_by}
         and
@@ -193,20 +200,26 @@ sub _adjust_select_args_for_complex_prefetch {
   my $outer_aliastypes =
     $self->_resolve_aliastypes_from_select_args( $from, $outer_select, $where, $outer_attrs );
 
+  # unroll parents
+  my ($outer_select_chain, $outer_restrict_chain) = map { +{
+    map { $_ => 1 } map { values %$_} map { @$_ } values %{ $outer_aliastypes->{$_} || {} }
+  } } qw/selecting restricting/;
+
   # see what's left - throw away if not selecting/restricting
-  # also throw in a group_by if restricting to guard against
-  # cross-join explosions
-  #
+  # also throw in a group_by if a non-selecting multiplier,
+  # to guard against cross-join explosions
   my $need_outer_group_by;
   while (my $j = shift @$from) {
     my $alias = $j->[0]{-alias};
 
-    if ($outer_aliastypes->{selecting}{$alias}) {
-      push @outer_from, $j;
+    if (
+      $outer_select_chain->{$alias}
+    ) {
+      push @outer_from, $j
     }
-    elsif ($outer_aliastypes->{restricting}{$alias}) {
+    elsif ($outer_restrict_chain->{$alias}) {
       push @outer_from, $j;
-      $need_outer_group_by ||= ! $j->[0]{-is_single};
+      $need_outer_group_by ||= $outer_aliastypes->{multiplying}{$alias} ? 1 : 0;
     }
   }
 
@@ -267,8 +280,13 @@ sub _resolve_aliastypes_from_select_args {
       or next;
 
     $alias_list->{$al} = $j;
-    $aliases_by_type->{multiplying}{$al} = 1
-      if ref($_) eq 'ARRAY' and ! $j->{-is_single}; # not array == {from} head == can't be multiplying
+    $aliases_by_type->{multiplying}{$al} ||= $j->{-join_path}||[] if (
+      # not array == {from} head == can't be multiplying
+      ( ref($_) eq 'ARRAY' and ! $j->{-is_single} )
+        or
+      # a parent of ours is already a multiplier
+      ( grep { $aliases_by_type->{multiplying}{$_} } @{ $j->{-join_path}||[] } )
+    );
   }
 
   # get a column to source/alias map (including unqualified ones)
@@ -329,7 +347,8 @@ sub _resolve_aliastypes_from_select_args {
 
     for my $type (keys %$to_scan) {
       for my $piece (@{$to_scan->{$type}}) {
-        $aliases_by_type->{$type}{$alias} = 1 if ($piece =~ $al_re);
+        $aliases_by_type->{$type}{$alias} ||= $alias_list->{$alias}{-join_path}||[]
+          if ($piece =~ $al_re);
       }
     }
   }
@@ -343,7 +362,10 @@ sub _resolve_aliastypes_from_select_args {
 
     for my $type (keys %$to_scan) {
       for my $piece (@{$to_scan->{$type}}) {
-        $aliases_by_type->{$type}{$colinfo->{$col}{-source_alias}} = 1 if ($piece =~ $col_re);
+        if ($piece =~ $col_re) {
+          my $alias = $colinfo->{$col}{-source_alias};
+          $aliases_by_type->{$type}{$alias} ||= $alias_list->{$alias}{-join_path}||[];
+        }
       }
     }
   }
@@ -351,22 +373,13 @@ sub _resolve_aliastypes_from_select_args {
   # Add any non-left joins to the restriction list (such joins are indeed restrictions)
   for my $j (values %$alias_list) {
     my $alias = $j->{-alias} or next;
-    $aliases_by_type->{restricting}{$alias} = 1 if (
+    $aliases_by_type->{restricting}{$alias} ||= $j->{-join_path}||[] if (
       (not $j->{-join_type})
         or
       ($j->{-join_type} !~ /^left (?: \s+ outer)? $/xi)
     );
   }
 
-  # mark all restricting/selecting join parents as such
-  # (e.g.  join => { cds => 'tracks' } - tracks will need to bring cds too )
-  for my $type (qw/restricting selecting/) {
-    for my $alias (keys %{$aliases_by_type->{$type}||{}}) {
-      $aliases_by_type->{$type}{$_} = 1
-        for (map { values %$_ } @{ $alias_list->{$alias}{-join_path} || [] });
-    }
-  }
-
   return $aliases_by_type;
 }
 

diff --git a/t/90join_torture.t b/t/90join_torture.t
index fb745dc..17d5116 100644 (file)
--- a/t/90join_torture.t
+++ b/t/90join_torture.t
@@ -154,4 +154,28 @@ lives_ok ( sub {
   ok (defined $rs->count);
 });
 
+# make sure multiplying endpoints do not lose heir join-path
+lives_ok (sub {
+  my $rs = $schema->resultset('CD')->search (
+    { },
+    { join => { artwork => 'images' } },
+  )->get_column('cdid');
+
+  is_same_sql_bind (
+    $rs->as_query,
+    '(
+      SELECT me.cdid
+        FROM cd me
+        LEFT JOIN cd_artwork artwork
+          ON artwork.cd_id = me.cdid
+        LEFT JOIN images images
+          ON images.artwork_id = artwork.cd_id
+    )',
+    [],
+  );
+
+  # execution
+  $rs->next;
+});
+
 done_testing;