From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 27 Feb 2013 10:19:43 +0000 (+0100)
Subject: An underdefined root has to use the %cur_row_ids container
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=06b3406df99bd27d739cf6de8f986c7b651bae4c;p=dbsrgits%2FDBIx-Class-Historic.git

An underdefined root has to use the %cur_row_ids container

Otherwise we are polluting $cur_row_data with the extra "virtual id column"
value, and may end up stashing it back on iterative cursor parsing
---

diff --git a/lib/DBIx/Class/ResultSource/RowParser/Util.pm b/lib/DBIx/Class/ResultSource/RowParser/Util.pm
index e89d723..83ec733 100644
--- a/lib/DBIx/Class/ResultSource/RowParser/Util.pm
+++ b/lib/DBIx/Class/ResultSource/RowParser/Util.pm
@@ -108,6 +108,9 @@ sub __visit_infmap_simple {
 sub assemble_collapsing_parser {
   my $args = shift;
 
+  # it may get unset further down
+  my $no_rowid_container = $args->{hri_style};
+
   my ($top_node_key, $top_node_key_assembler);
 
   if (scalar @{$args->{collapse_map}{-identifying_columns}}) {
@@ -138,6 +141,7 @@ sub assemble_collapsing_parser {
       -custom_node_key => $top_node_key,
     };
 
+    $no_rowid_container = 0;
   }
   else {
     die('Unexpected collapse map contents');
@@ -145,7 +149,7 @@ sub assemble_collapsing_parser {
 
   my ($data_assemblers, $stats) = __visit_infmap_collapse ($args);
 
-  my @idcol_args = $args->{hri_style} ? ('', '') : (
+  my @idcol_args = $no_rowid_container ? ('', '') : (
     ', %cur_row_ids', # only declare the variable if we'll use it
 
     sprintf( <<'EOS', join ', ', sort { $a <=> $b } keys %{ $stats->{idcols_seen} } ),
@@ -199,7 +203,11 @@ EOS
   # !!! note - different var than the one above
   # change the quoted placeholders to unquoted alias-references
   $parser_src =~ s/ \' \xFF__VALPOS__(\d+)__\xFF \' /"\$cur_row_data->[$1]"/gex;
-  $parser_src =~ s/ \' \xFF__IDVALPOS__(\d+)__\xFF \' /$args->{hri_style} ? "\$cur_row_data->[$1]" : "\$cur_row_ids{$1}" /gex;
+  $parser_src =~ s/
+    \' \xFF__IDVALPOS__(\d+)__\xFF \'
+  /
+    $no_rowid_container ? "\$cur_row_data->[$1]" : "\$cur_row_ids{$1}"
+  /gex;
 
   $parser_src = "  { use strict; use warnings FATAL => 'all';\n$parser_src\n  }";
 }
diff --git a/t/resultset/rowparser_internals.t b/t/resultset/rowparser_internals.t
index c7d8ac8..03d1b28 100644
--- a/t/resultset/rowparser_internals.t
+++ b/t/resultset/rowparser_internals.t
@@ -622,7 +622,7 @@ is_same_src (
     hri_style => 1,
   }),
   ' my $rows_pos = 0;
-    my ($result_pos, @collapse_idx, $cur_row_data);
+    my ($result_pos, @collapse_idx, $cur_row_data, %cur_row_ids);
 
     while ($cur_row_data = (
       ( $rows_pos >= 0 and $_[0][$rows_pos++] ) or do { $rows_pos = -1; undef } )
@@ -630,8 +630,11 @@ is_same_src (
       ( $_[1] and $_[1]->() )
     ) {
 
+      $cur_row_ids{$_} = defined $$cur_row_data[$_] ? $$cur_row_data[$_] : "\0NULL\xFF$rows_pos\xFF$_\0"
+        for (0, 2, 3, 4, 8);
+
       # cache expensive set of ops in a non-existent rowid slot
-      $cur_row_data->[10] = (
+      $cur_row_ids{10} = (
         ( ( defined $cur_row_data->[0] ) && (join "\xFF", q{}, $cur_row_data->[0], q{} ))
           or
         ( ( defined $cur_row_data->[2] ) && (join "\xFF", q{}, $cur_row_data->[2], q{} ))
@@ -640,42 +643,42 @@ is_same_src (
       );
 
       # a present cref in $_[1] implies lazy prefetch, implies a supplied stash in $_[2]
-      $_[1] and $result_pos and ! $collapse_idx[0]{$cur_row_data->[10]} and (unshift @{$_[2]}, $cur_row_data) and last;
+      $_[1] and $result_pos and ! $collapse_idx[0]{$cur_row_ids{10}} and (unshift @{$_[2]}, $cur_row_data) and last;
 
-      $collapse_idx[0]{$cur_row_data->[10]} ||= $_[0][$result_pos++] = { year => $$cur_row_data[1] };
+      $collapse_idx[0]{$cur_row_ids{10}} ||= $_[0][$result_pos++] = { year => $$cur_row_data[1] };
 
-      (! defined $cur_row_data->[0] ) ? $collapse_idx[0]{$cur_row_data->[10]}{single_track} = undef : do {
+      (! defined $cur_row_data->[0] ) ? $collapse_idx[0]{$cur_row_ids{10}}{single_track} = undef : do {
 
-        $collapse_idx[0]{$cur_row_data->[10]}{single_track} ||= ($collapse_idx[1]{$cur_row_data->[0]} = { trackid => $$cur_row_data[0] });
+        $collapse_idx[0]{$cur_row_ids{10}}{single_track} ||= ($collapse_idx[1]{$cur_row_ids{0}} = { trackid => $$cur_row_data[0] });
 
-        $collapse_idx[1]{$cur_row_data->[0]}{cd} ||= $collapse_idx[2]{$cur_row_data->[0]};
+        $collapse_idx[1]{$cur_row_ids{0}}{cd} ||= $collapse_idx[2]{$cur_row_ids{0}};
 
-        $collapse_idx[2]{$cur_row_data->[0]}{artist} ||= ($collapse_idx[3]{$cur_row_data->[0]} = { artistid => $$cur_row_data[6] });
+        $collapse_idx[2]{$cur_row_ids{0}}{artist} ||= ($collapse_idx[3]{$cur_row_ids{0}} = { artistid => $$cur_row_data[6] });
 
-        (! defined $cur_row_data->[4] ) ? $collapse_idx[3]{$cur_row_data->[0]}{cds} = [] : do {
+        (! defined $cur_row_data->[4] ) ? $collapse_idx[3]{$cur_row_ids{0}}{cds} = [] : do {
 
-          (! $collapse_idx[4]{$cur_row_data->[0]}{$cur_row_data->[4]} )
+          (! $collapse_idx[4]{$cur_row_ids{0}}{$cur_row_ids{4}} )
             and
-          push @{$collapse_idx[3]{$cur_row_data->[0]}{cds}}, (
-              $collapse_idx[4]{$cur_row_data->[0]}{$cur_row_data->[4]} = { cdid => $$cur_row_data[4], genreid => $$cur_row_data[7], year => $$cur_row_data[5] }
+          push @{$collapse_idx[3]{$cur_row_ids{0}}{cds}}, (
+              $collapse_idx[4]{$cur_row_ids{0}}{$cur_row_ids{4}} = { cdid => $$cur_row_data[4], genreid => $$cur_row_data[7], year => $$cur_row_data[5] }
           );
 
-          (! defined $cur_row_data->[8] ) ? $collapse_idx[4]{$cur_row_data->[0]}{$cur_row_data->[4]}{tracks} = [] : do {
+          (! defined $cur_row_data->[8] ) ? $collapse_idx[4]{$cur_row_ids{0}}{$cur_row_ids{4}}{tracks} = [] : do {
 
-            (! $collapse_idx[5]{$cur_row_data->[0]}{$cur_row_data->[4]}{$cur_row_data->[8]} )
+            (! $collapse_idx[5]{$cur_row_ids{0}}{$cur_row_ids{4}}{$cur_row_ids{8}} )
               and
-            push @{$collapse_idx[4]{$cur_row_data->[0]}{$cur_row_data->[4]}{tracks}}, (
-                $collapse_idx[5]{$cur_row_data->[0]}{$cur_row_data->[4]}{$cur_row_data->[8]} = { title => $$cur_row_data[8] }
+            push @{$collapse_idx[4]{$cur_row_ids{0}}{$cur_row_ids{4}}{tracks}}, (
+                $collapse_idx[5]{$cur_row_ids{0}}{$cur_row_ids{4}}{$cur_row_ids{8}} = { title => $$cur_row_data[8] }
             );
           };
         };
       };
 
-      (! defined $cur_row_data->[2] ) ? $collapse_idx[0]{$cur_row_data->[10]}{tracks} = [] : do {
-        (! $collapse_idx[6]{$cur_row_data->[2]}{$cur_row_data->[3]} )
+      (! defined $cur_row_data->[2] ) ? $collapse_idx[0]{$cur_row_ids{10}}{tracks} = [] : do {
+        (! $collapse_idx[6]{$cur_row_ids{2}}{$cur_row_ids{3}} )
           and
-        push @{$collapse_idx[0]{$cur_row_data->[10]}{tracks}}, (
-            $collapse_idx[6]{$cur_row_data->[2]}{$cur_row_data->[3]} = { cd => $$cur_row_data[2], title => $$cur_row_data[3] }
+        push @{$collapse_idx[0]{$cur_row_ids{10}}{tracks}}, (
+            $collapse_idx[6]{$cur_row_ids{2}}{$cur_row_ids{3}} = { cd => $$cur_row_data[2], title => $$cur_row_data[3] }
         );
       };
     }