From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 10 Feb 2013 09:15:06 +0000 (+0100)
Subject: Yet another collapser oversight
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=3d8caf63b7fa6f2adf9bbd1f49bc7ad932d031b6;p=dbsrgits%2FDBIx-Class-Historic.git

Yet another collapser oversight

One can't reuse collapsers across a left join
---

diff --git a/lib/DBIx/Class/ResultSource/RowParser.pm b/lib/DBIx/Class/ResultSource/RowParser.pm
index c0e0d97..cc4706c 100644
--- a/lib/DBIx/Class/ResultSource/RowParser.pm
+++ b/lib/DBIx/Class/ResultSource/RowParser.pm
@@ -400,7 +400,13 @@ sub _resolve_collapse {
 
         # if this is a 1:1 our own collapser can be used as a collapse-map
         # (regardless of left or not)
-        collapser_reusable => @{$collapse_map->{-identifying_columns}} && $relinfo->{$rel}{is_single},
+        collapser_reusable => (
+          $relinfo->{$rel}{is_single}
+            &&
+          $relinfo->{$rel}{is_inner}
+            &&
+          @{$collapse_map->{-identifying_columns}}
+        ) ? 1 : 0,
       },
     }, $common_args );
 
diff --git a/t/lib/DBICTest/Schema/Lyrics.pm b/t/lib/DBICTest/Schema/Lyrics.pm
index bb0a56b..3009314 100644
--- a/t/lib/DBICTest/Schema/Lyrics.pm
+++ b/t/lib/DBICTest/Schema/Lyrics.pm
@@ -21,4 +21,8 @@ __PACKAGE__->set_primary_key('lyric_id');
 __PACKAGE__->belongs_to('track', 'DBICTest::Schema::Track', 'track_id');
 __PACKAGE__->has_many('lyric_versions', 'DBICTest::Schema::LyricVersion', 'lyric_id');
 
+__PACKAGE__->has_many('existing_lyric_versions', 'DBICTest::Schema::LyricVersion', 'lyric_id', {
+  join_type => 'inner',
+});
+
 1;
diff --git a/t/resultset/rowparser_internals.t b/t/resultset/rowparser_internals.t
index fec5656..9d71d4c 100644
--- a/t/resultset/rowparser_internals.t
+++ b/t/resultset/rowparser_internals.t
@@ -152,7 +152,7 @@ is_same_src (
 );
 
 $infmap = [qw/
-  tracks.lyrics.lyric_versions.text
+  tracks.lyrics.existing_lyric_versions.text
   existing_single_track.cd.artist.artistid
   existing_single_track.cd.artist.cds.year
   year
@@ -162,6 +162,7 @@ $infmap = [qw/
   latest_cd
   existing_single_track.cd.artist.cds.tracks.title
   existing_single_track.cd.artist.cds.genreid
+  tracks.lyrics.existing_lyric_versions.lyric_id
 /];
 
 is_deeply (
@@ -198,13 +199,12 @@ is_deeply (
       -is_optional => 1,
 
       lyrics => {
-        -identifying_columns => [ 1, 5 ], # existing_single_track.cd.artist.artistid, tracks.title
+        -identifying_columns => [ 1, 5, 10 ], # existing_single_track.cd.artist.artistid, tracks.title, tracks.lyrics.existing_lyric_versions.lyric_id
         -is_single => 1,
         -is_optional => 1,
 
-        lyric_versions => {
-          -identifying_columns => [ 0, 1, 5 ], # tracks.lyrics.lyric_versions.text, existing_single_track.cd.artist.artistid, tracks.title
-          -is_optional => 1,
+        existing_lyric_versions => {
+          -identifying_columns => [ 0, 1, 5, 10 ], # tracks.lyrics.existing_lyric_versions.text, existing_single_track.cd.artist.artistid, tracks.title, tracks.lyrics.existing_lyric_versions.lyric_id
         },
       },
     }
@@ -226,7 +226,7 @@ is_same_src (
     ) {
 
       $cur_row_ids{$_} = defined $cur_row_data->[$_] ? $cur_row_data->[$_] : "\0NULL\xFF$rows_pos\xFF$_\0"
-        for (0, 1, 5, 6, 8);
+        for (0, 1, 5, 6, 8, 10);
 
       # a present cref in $_[1] implies lazy prefetch, implies a supplied stash in $_[2]
       $_[1] and $result_pos and unshift(@{$_[2]}, $cur_row_data) and last
@@ -247,10 +247,10 @@ is_same_src (
       push @{ $collapse_idx[0]{$cur_row_ids{1}}[1]{tracks} }, $collapse_idx[6]{$cur_row_ids{1}}{$cur_row_ids{5}} ||= [{ title => $cur_row_data->[5] }]
         unless $collapse_idx[6]{$cur_row_ids{1}}{$cur_row_ids{5}};
 
-      $collapse_idx[6]{$cur_row_ids{1}}{$cur_row_ids{5}}[1]{lyrics} ||= $collapse_idx[7]{$cur_row_ids{1}}{$cur_row_ids{5} };
+      $collapse_idx[6]{$cur_row_ids{1}}{$cur_row_ids{5}}[1]{lyrics} ||= $collapse_idx[7]{$cur_row_ids{1}}{$cur_row_ids{5}}{$cur_row_ids{10}};
 
-      push @{ $collapse_idx[7]{$cur_row_ids{1}}{$cur_row_ids{5}}[1]{lyric_versions} }, $collapse_idx[8]{$cur_row_ids{0}}{$cur_row_ids{1}}{$cur_row_ids{5}} ||= [{ text => $cur_row_data->[0] }]
-        unless $collapse_idx[8]{$cur_row_ids{0}}{$cur_row_ids{1}}{$cur_row_ids{5}};
+      push @{ $collapse_idx[7]{$cur_row_ids{1}}{$cur_row_ids{5}}{$cur_row_ids{10}}[1]{existing_lyric_versions} }, $collapse_idx[8]{$cur_row_ids{0}}{$cur_row_ids{1}}{$cur_row_ids{5}}{$cur_row_ids{10}} ||= [{ lyric_id => $cur_row_data->[10], text => $cur_row_data->[0] }]
+        unless $collapse_idx[8]{$cur_row_ids{0}}{$cur_row_ids{1}}{$cur_row_ids{5}}{$cur_row_ids{10}};
 
       $_[0][$result_pos++] = $collapse_idx[0]{$cur_row_ids{1}}
         if $is_new_res;