Fix _resolve_collapse to always consider the entire chain of parent ids
Peter Rabbitson [Mon, 28 Jan 2013 11:20:45 +0000 (12:20 +0100)]
Instead of relying on a "local column based" resolution for a particular
node, always add all the "collapsers" of the left-side "parents".

In the process cleanup the return of _resolve_collapse, and make it more
generic (after all we are hoping to CPAN the collapser generator some
day).

The change above  solves the following problem: think of an artist with
3 CDs A, B & C:

$cd_rs->search(
  [
    { 'me.title' => 'A', 'cds.title' => { '!=' => 'B' } },
    { 'me.title' => { '!=', => 'A' }, 'cds.title' => 'B' },
  ],
  { prefetch => { artist => 'cds' } },
);

Since an artist is fully defined by its PK (which we already fetch),
an all() run will keep accumulating all 3 CDs under the same already
known hash, representing said artist, because we do not create two
distinct pigeon-hole-hashes for "artist of A" and "artist of non-A".

lib/DBIx/Class/ResultSource/RowParser.pm
t/resultset/rowparser_internals.t

index 1d1e781..c74ab9e 100644 (file)
@@ -142,7 +142,7 @@ sub _resolve_collapse {
       values %{$args->{_parent_info}{rel_condition} || {}}
     };
 
-    $my_cols->{$_} = { via_collapse => $args->{_parent_info}{collapse_on} }
+    $my_cols->{$_} = { via_collapse => $args->{_parent_info}{collapse_on_idcols} }
       for keys %{$assumed_from_parent->{columns}};
   }
 
@@ -154,8 +154,18 @@ sub _resolve_collapse {
 
   my $collapse_map;
 
-  # try to resolve based on our columns (plus already inserted FK bridges)
+  # first try to reuse the parent's collapser (i.e. reuse collapser over 1:1)
+  # (makes for a leaner coderef later)
+  unless ($collapse_map->{-idcols_current_node}) {
+    $collapse_map->{-idcols_current_node} = $args->{_parent_info}{collapse_on_idcols}
+      if $args->{_parent_info}{collapser_reusable};
+  }
+
+
+  # Still dont know how to collapse - try to resolve based on our columns (plus already inserted FK bridges)
   if (
+    ! $collapse_map->{-idcols_current_node}
+      and
     $my_cols
       and
     my $idset = $self->_identifying_column_set ({map { $_ => $my_cols->{$_}{colinfo} } keys %$my_cols})
@@ -164,8 +174,9 @@ sub _resolve_collapse {
     # and fix stuff up if this is the case
     my @reduced_set = grep { ! $assumed_from_parent->{columns}{$_} } @$idset;
 
-    $collapse_map->{-node_id} = __unique_numlist(
-      (@reduced_set != @$idset) ? @{$args->{_parent_info}{collapse_on}} : (),
+    $collapse_map->{-idcols_current_node} = [ __unique_numlist(
+      @{ $args->{_parent_info}{collapse_on_idcols}||[] },
+
       (map
         {
           my $fqc = join ('.',
@@ -177,13 +188,13 @@ sub _resolve_collapse {
         }
         @reduced_set
       ),
-    );
+    )];
   }
 
   # Stil don't know how to collapse - keep descending down 1:1 chains - if
   # a related non-LEFT 1:1 is resolvable - its condition will collapse us
   # too
-  unless ($collapse_map->{-node_id}) {
+  unless ($collapse_map->{-idcols_current_node}) {
     my @candidates;
 
     for my $rel (keys %$relinfo) {
@@ -194,7 +205,7 @@ sub _resolve_collapse {
         _rel_chain => [ @{$args->{_rel_chain}}, $rel ],
         _parent_info => { underdefined => 1 },
       }, $common_args)) {
-        push @candidates, $rel_collapse->{-node_id};
+        push @candidates, $rel_collapse->{-idcols_current_node};
       }
     }
 
@@ -202,25 +213,18 @@ sub _resolve_collapse {
     # FIXME - maybe need to implement a data type order as well (i.e. prefer several ints
     # to a single varchar)
     if (@candidates) {
-      ($collapse_map->{-node_id}) = sort { scalar @$a <=> scalar @$b } (@candidates);
+      ($collapse_map->{-idcols_current_node}) = sort { scalar @$a <=> scalar @$b } (@candidates);
     }
   }
 
-  # Still dont know how to collapse - see if the parent passed us anything
-  # (i.e. reuse collapser over 1:1)
-  unless ($collapse_map->{-node_id}) {
-    $collapse_map->{-node_id} = $args->{_parent_info}{collapse_on}
-      if $args->{_parent_info}{collapser_reusable};
-  }
-
   # stop descending into children if we were called by a parent for first-pass
   # and don't despair if nothing was found (there may be other parallel branches
   # to dive into)
   if ($args->{_parent_info}{underdefined}) {
-    return $collapse_map->{-node_id} ? $collapse_map : undef
+    return $collapse_map->{-idcols_current_node} ? $collapse_map : undef
   }
   # nothing down the chain resolved - can't calculate a collapse-map
-  elsif (! $collapse_map->{-node_id}) {
+  elsif (! $collapse_map->{-idcols_current_node}) {
     $self->throw_exception ( sprintf
       "Unable to calculate a definitive collapse column set for %s%s: fetch more unique non-nullable columns",
       $self->source_name,
@@ -237,14 +241,16 @@ sub _resolve_collapse {
   $collapse_map->{-is_optional} = 1 if $args->{_parent_info}{is_optional};
   $collapse_map->{-node_index} = $common_args->{_node_idx}++;
 
-  my (@id_sets, $multis_in_chain);
+
+  my @id_sets;
   for my $rel (sort keys %$relinfo) {
 
     $collapse_map->{$rel} = $relinfo->{$rel}{rsrc}->_resolve_collapse ({
       as => { map { $_ => 1 } ( keys %{$rel_cols->{$rel}} ) },
       _rel_chain => [ @{$args->{_rel_chain}}, $rel],
       _parent_info => {
-        collapse_on => [ @{$collapse_map->{-node_id}} ],
+        # shallow copy
+        collapse_on_idcols => [ @{$collapse_map->{-idcols_current_node}} ],
 
         rel_condition => $relinfo->{$rel}{fk_map},
 
@@ -258,10 +264,19 @@ sub _resolve_collapse {
 
     $collapse_map->{$rel}{-is_single} = 1 if $relinfo->{$rel}{is_single};
     $collapse_map->{$rel}{-is_optional} ||= 1 unless $relinfo->{$rel}{is_inner};
-    push @id_sets, @{ $collapse_map->{$rel}{-branch_id} };
+    push @id_sets, ( map { @$_ } (
+      $collapse_map->{$rel}{-idcols_current_node},
+      $collapse_map->{$rel}{-idcols_extra_from_children} || (),
+    ));
   }
 
-  $collapse_map->{-branch_id} = __unique_numlist( @id_sets, @{$collapse_map->{-node_id}} );
+  if (@id_sets) {
+    my $cur_nodeid_hash = { map { $_ => 1 } @{$collapse_map->{-idcols_current_node}} };
+    $collapse_map->{-idcols_extra_from_children} = [ grep
+      { ! $cur_nodeid_hash->{$_} }
+      __unique_numlist( @id_sets )
+    ];
+  }
 
   return $collapse_map;
 }
@@ -322,7 +337,6 @@ sub _mk_row_parser {
   # the collapsing parser is more complicated - it needs to keep a lot of state
   #
   else {
-
     my $collapse_map = $self->_resolve_collapse ({
       # FIXME
       # only consider real columns (not functions) during collapse resolution
@@ -338,19 +352,22 @@ sub _mk_row_parser {
       }
     });
 
-    my $top_branch_idx_list = join (', ', @{$collapse_map->{-branch_id}});
+    my $all_idcols_as_list = join ', ', sort map { @$_ } (
+      $collapse_map->{-idcols_current_node},
+      $collapse_map->{-idcols_extra_from_children} || (),
+    );
 
     my $top_node_id_path = join ('', map
       { "{'\xFF__IDVALPOS__${_}__\xFF'}" }
-      @{$collapse_map->{-node_id}}
+      @{$collapse_map->{-idcols_current_node}}
     );
 
     my $rel_assemblers = __visit_infmap_collapse (
       $inflate_index, $collapse_map
     );
 
-    $parser_src = sprintf (<<'EOS', $top_branch_idx_list, $top_node_id_path, $rel_assemblers);
-### BEGIN STRING EVAL
+    $parser_src = sprintf (<<'EOS', $all_idcols_as_list, $top_node_id_path, $rel_assemblers);
+### BEGIN LITERAL STRING EVAL
 
   my ($rows_pos, $result_pos, $cur_row, @cur_row_ids, @collapse_idx, $is_new_res) = (0,0);
 
@@ -366,8 +383,10 @@ sub _mk_row_parser {
     ($_[1] and $_[1]->())
   ) {
 
+    # due to left joins some of the ids may be NULL/undef, and
+    # won't play well when used as hash lookups
     $cur_row_ids[$_] = defined $cur_row->[$_] ? $cur_row->[$_] : "\xFF\xFFN\xFFU\xFFL\xFFL\xFF\xFF"
-      for (%1$s); # the top branch_id includes all id values
+      for (%1$s);
 
     $is_new_res = ! $collapse_idx[1]%2$s and (
       $_[1] and $result_pos and (unshift @{$_[2]}, $cur_row) and last
@@ -380,7 +399,7 @@ sub _mk_row_parser {
   }
 
   splice @{$_[0]}, $result_pos; # truncate the passed in array for cases of collapsing ->all()
-### END STRING EVAL
+### END LITERAL STRING EVAL
 EOS
 
     # !!! note - different var than the one above
@@ -462,7 +481,7 @@ sub __visit_infmap_collapse {
 
   my $sequenced_node_id = join ('', map
     { "{'\xFF__IDVALPOS__${_}__\xFF'}" }
-    @{$collapse_map->{-node_id}}
+    @{$collapse_map->{-idcols_current_node}}
   );
 
   my $me_struct = keys %$my_cols
@@ -501,12 +520,13 @@ sub __visit_infmap_collapse {
 
   # DISABLEPRUNE
   #my $known_defined = { %{ $parent_info->{known_defined} || {} } };
-  #$known_defined->{$_}++ for @{$collapse_map->{-node_id}};
+  #$known_defined->{$_}++ for @{$collapse_map->{-idcols_current_node}};
 
   for my $rel (sort keys %$rel_cols) {
 
-    push @src, sprintf( '%s[1]{%s} ||= [];', $node_idx_ref, perlstring($rel) )
-      unless $collapse_map->{$rel}{-is_single};
+#    push @src, sprintf(
+#      '%s[1]{%s} ||= [];', $node_idx_ref, perlstring($rel)
+#    ) unless $collapse_map->{$rel}{-is_single};
 
     push @src,  __visit_infmap_collapse($rel_cols->{$rel}, $collapse_map->{$rel}, {
       node_idx => $collapse_map->{-node_index},
@@ -521,7 +541,7 @@ sub __visit_infmap_collapse {
     #  { "(! defined '\xFF__IDVALPOS__${_}__\xFF')" }
     #  sort { $a <=> $b } grep
     #    { ! $known_defined->{$_} }
-    #    @{$collapse_map->{$rel}{-node_id}}
+    #    @{$collapse_map->{$rel}{-idcols_current_node}}
     #) {
     #  $src[-1] = sprintf( '(%s) or %s',
     #    join (' || ', @null_checks ),
@@ -535,7 +555,7 @@ sub __visit_infmap_collapse {
 
 # adding a dep on MoreUtils *just* for this is retarded
 sub __unique_numlist {
-  [ sort { $a <=> $b } keys %{ {map { $_ => 1 } @_ }} ]
+  sort { $a <=> $b } keys %{ {map { $_ => 1 } @_ }}
 }
 
 # This error must be thrown from two distinct codepaths, joining them is
index 7a03287..60bf1ba 100644 (file)
@@ -37,9 +37,9 @@ is_same_src (
 );
 
 $infmap = [qw/
+  single_track.cd.artist.cds.tracks.title
   single_track.cd.artist.artistid
   year
-  single_track.cd.artist.cds.tracks.title
   single_track.cd.artist.cds.cdid
   title
   artist
@@ -49,17 +49,17 @@ is_same_src (
     inflate_map => $infmap,
   }),
   '$_ = [
-    { artist => $_->[5], title => $_->[4], year => $_->[1] },
+    { artist => $_->[5], title => $_->[4], year => $_->[2] },
     { single_track => [
       undef,
       { cd => [
         undef,
         { artist => [
-          { artistid => $_->[0] },
+          { artistid => $_->[1] },
           { cds => [
             { cdid => $_->[3] },
             { tracks => [
-              { title => $_->[2] }
+              { title => $_->[0] }
             ] },
           ] },
         ] },
@@ -70,41 +70,40 @@ is_same_src (
 );
 
 is_deeply (
-  $schema->source('CD')->_resolve_collapse({ as => {map { $infmap->[$_] => $_ } 0 .. $#$infmap} }),
+  ($schema->source('CD')->_resolve_collapse({ as => {map { $infmap->[$_] => $_ } 0 .. $#$infmap} })),
   {
     -node_index => 1,
-    -node_id => [ 4, 5 ],
-    -branch_id => [ 0, 2, 3, 4, 5 ],
+    -idcols_current_node => [ 4, 5 ],
+    -idcols_extra_from_children => [ 0, 3 ],
 
     single_track => {
       -node_index => 2,
-      -node_id => [ 4, 5],
-      -branch_id => [ 0, 2, 3, 4, 5],
+      -idcols_current_node => [ 4, 5 ],
+      -idcols_extra_from_children => [ 0, 3 ],
       -is_optional => 1,
       -is_single => 1,
 
       cd => {
         -node_index => 3,
-        -node_id => [ 4, 5 ],
-        -branch_id => [ 0, 2, 3, 4, 5 ],
+        -idcols_current_node => [ 4, 5 ],
+        -idcols_extra_from_children => [ 0, 3 ],
         -is_single => 1,
 
         artist => {
           -node_index => 4,
-          -node_id => [ 0 ],
-          -branch_id => [ 0, 2, 3 ],
+          -idcols_current_node => [ 4, 5 ],
+          -idcols_extra_from_children => [ 0, 3 ],
           -is_single => 1,
 
           cds => {
             -node_index => 5,
-            -node_id => [ 3 ],
-            -branch_id => [ 2, 3 ],
+            -idcols_current_node => [ 3, 4, 5 ],
+            -idcols_extra_from_children => [ 0 ],
             -is_optional => 1,
 
             tracks => {
               -node_index => 6,
-              -node_id => [ 2, 3 ],
-              -branch_id => [ 2, 3 ],
+              -idcols_current_node => [ 0, 3, 4, 5 ],
               -is_optional => 1,
             },
           },
@@ -129,24 +128,31 @@ is_same_src (
     ) {
 
       $cur_row_ids[$_] = defined $cur_row->[$_] ? $cur_row->[$_] : "\xFF\xFFN\xFFU\xFFL\xFFL\xFF\xFF"
-        for (0, 2, 3, 4, 5);
+        for (0, 3, 4, 5);
 
-      # a present cref implies lazy prefetch, implies a supplied stash in $_[2]
+      # a present cref in $_[1] implies lazy prefetch, implies a supplied stash in $_[2]
       $_[1] and $result_pos and unshift(@{$_[2]}, $cur_row) and last
         if $is_new_res = ! $collapse_idx[1]{$cur_row_ids[4]}{$cur_row_ids[5]};
 
-      $collapse_idx[1]{$cur_row_ids[4]}{$cur_row_ids[5]} ||= [{ artist => $cur_row->[5], title => $cur_row->[4], year => $cur_row->[1] }];
+      # the rowdata itself for root node
+      $collapse_idx[1]{$cur_row_ids[4]}{$cur_row_ids[5]} ||= [{ artist => $cur_row->[5], title => $cur_row->[4], year => $cur_row->[2] }];
+
+      # prefetch data of single_track (placed in root)
       $collapse_idx[1]{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{single_track} ||= $collapse_idx[2]{$cur_row_ids[4]}{$cur_row_ids[5]};
+
+      # prefetch data of cd (placed in single_track)
       $collapse_idx[2]{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{cd} ||= $collapse_idx[3]{$cur_row_ids[4]}{$cur_row_ids[5]};
-      $collapse_idx[3]{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{artist} ||= $collapse_idx[4]{$cur_row_ids[0]} ||= [{ artistid => $cur_row->[0] }];
 
-      $collapse_idx[4]{$cur_row_ids[0]}[1]{cds} ||= [];
-      push @{$collapse_idx[4]{$cur_row_ids[0]}[1]{cds}}, $collapse_idx[5]{$cur_row_ids[3]} ||= [{ cdid => $cur_row->[3] }]
-        unless $collapse_idx[5]{$cur_row_ids[3]};
+      # prefetch data of artist ( placed in single_track->cd)
+      $collapse_idx[3]{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{artist} ||= $collapse_idx[4]{$cur_row_ids[4]}{$cur_row_ids[5]} ||= [{ artistid => $cur_row->[1] }];
+
+      # prefetch data of cds (if available)
+      push @{$collapse_idx[4]{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{cds}}, $collapse_idx[5]{$cur_row_ids[3]}{$cur_row_ids[4]}{$cur_row_ids[5]} ||= [{ cdid => $cur_row->[3] }]
+        unless $collapse_idx[5]{$cur_row_ids[3]}{$cur_row_ids[4]}{$cur_row_ids[5]};
 
-      $collapse_idx[5]{$cur_row_ids[3]}[1]{tracks} ||= [];
-      push @{$collapse_idx[5]{$cur_row_ids[3]}[1]{tracks}}, $collapse_idx[6]{$cur_row_ids[2]}{$cur_row_ids[3]} ||= [{ title => $cur_row->[2] }]
-        unless $collapse_idx[6]{$cur_row_ids[2]}{$cur_row_ids[3]};
+      # prefetch data of tracks (if available)
+      push @{$collapse_idx[5]{$cur_row_ids[3]}{$cur_row_ids[4]}{$cur_row_ids[5]}[1]{tracks}}, $collapse_idx[6]{$cur_row_ids[0]}{$cur_row_ids[3]}{$cur_row_ids[4]}{$cur_row_ids[5]} ||= [{ title => $cur_row->[0] }]
+        unless $collapse_idx[6]{$cur_row_ids[0]}{$cur_row_ids[3]}{$cur_row_ids[4]}{$cur_row_ids[5]};
 
       $_[0][$result_pos++] = $collapse_idx[1]{$cur_row_ids[4]}{$cur_row_ids[5]}
         if $is_new_res;
@@ -173,37 +179,36 @@ is_deeply (
   $schema->source('CD')->_resolve_collapse({ as => {map { $infmap->[$_] => $_ } 0 .. $#$infmap} }),
   {
     -node_index => 1,
-    -node_id => [ 1 ], # existing_single_track.cd.artist.artistid
-    -branch_id => [ 0, 1, 5, 6, 8 ],
+    -idcols_current_node => [ 1 ], # existing_single_track.cd.artist.artistid
+    -idcols_extra_from_children => [ 0, 5, 6, 8 ],
 
     existing_single_track => {
       -node_index => 2,
-      -node_id => [ 1 ], # existing_single_track.cd.artist.artistid
-      -branch_id => [ 1, 6, 8 ],
+      -idcols_current_node => [ 1 ], # existing_single_track.cd.artist.artistid
+      -idcols_extra_from_children => [ 6, 8 ],
       -is_single => 1,
 
       cd => {
         -node_index => 3,
-        -node_id => [ 1 ], # existing_single_track.cd.artist.artistid
-        -branch_id => [ 1, 6, 8 ],
+        -idcols_current_node => [ 1 ], # existing_single_track.cd.artist.artistid
+        -idcols_extra_from_children => [ 6, 8 ],
         -is_single => 1,
 
         artist => {
           -node_index => 4,
-          -node_id => [ 1 ], # existing_single_track.cd.artist.artistid
-          -branch_id => [ 1, 6, 8 ],
+          -idcols_current_node => [ 1 ], # existing_single_track.cd.artist.artistid
+          -idcols_extra_from_children => [ 6, 8 ],
           -is_single => 1,
 
           cds => {
             -node_index => 5,
-            -node_id => [ 6 ], # existing_single_track.cd.artist.cds.cdid
-            -branch_id => [ 6, 8 ],
+            -idcols_current_node => [ 1, 6 ], # existing_single_track.cd.artist.cds.cdid
+            -idcols_extra_from_children => [ 8 ],
             -is_optional => 1,
 
             tracks => {
               -node_index => 6,
-              -node_id => [ 6, 8 ], # existing_single_track.cd.artist.cds.cdid, existing_single_track.cd.artist.cds.tracks.title
-              -branch_id => [ 6, 8 ],
+              -idcols_current_node => [ 1, 6, 8 ], # existing_single_track.cd.artist.cds.cdid, existing_single_track.cd.artist.cds.tracks.title
               -is_optional => 1,
             }
           }
@@ -212,21 +217,20 @@ is_deeply (
     },
     tracks => {
       -node_index => 7,
-      -node_id => [ 1, 5 ], # existing_single_track.cd.artist.artistid, tracks.title
-      -branch_id => [ 0, 1, 5 ],
+      -idcols_current_node => [ 1, 5 ], # existing_single_track.cd.artist.artistid, tracks.title
+      -idcols_extra_from_children => [ 0 ],
       -is_optional => 1,
 
       lyrics => {
         -node_index => 8,
-        -node_id => [ 1, 5 ], # existing_single_track.cd.artist.artistid, tracks.title
-        -branch_id => [ 0, 1, 5 ],
+        -idcols_current_node => [ 1, 5 ], # existing_single_track.cd.artist.artistid, tracks.title
+        -idcols_extra_from_children => [ 0 ],
         -is_single => 1,
         -is_optional => 1,
 
         lyric_versions => {
           -node_index => 9,
-          -node_id => [ 0, 1, 5 ], # tracks.lyrics.lyric_versions.text, existing_single_track.cd.artist.artistid, tracks.title
-          -branch_id => [ 0, 1, 5 ],
+          -idcols_current_node => [ 0, 1, 5 ], # tracks.lyrics.lyric_versions.text, existing_single_track.cd.artist.artistid, tracks.title
           -is_optional => 1,
         },
       },
@@ -261,21 +265,17 @@ is_same_src (
       $collapse_idx[2]{$cur_row_ids[1]}[1]{cd} ||= $collapse_idx[3]{$cur_row_ids[1]};
       $collapse_idx[3]{$cur_row_ids[1]}[1]{artist} ||= $collapse_idx[4]{$cur_row_ids[1]} ||= [{ artistid => $cur_row->[1] }];
 
-      $collapse_idx[4]{$cur_row_ids[1]}[1]{cds} ||= [];
-      push @{ $collapse_idx[4]{$cur_row_ids[1]}[1]{cds} }, $collapse_idx[5]{$cur_row_ids[6]} ||= [{ cdid => $cur_row->[6], genreid => $cur_row->[9], year => $cur_row->[2] }]
-        unless $collapse_idx[5]{$cur_row_ids[6]};
+      push @{ $collapse_idx[4]{$cur_row_ids[1]}[1]{cds} }, $collapse_idx[5]{$cur_row_ids[1]}{$cur_row_ids[6]} ||= [{ cdid => $cur_row->[6], genreid => $cur_row->[9], year => $cur_row->[2] }]
+        unless $collapse_idx[5]{$cur_row_ids[1]}{$cur_row_ids[6]};
 
-      $collapse_idx[5]{$cur_row_ids[6]}[1]{tracks} ||= [];
-      push @{ $collapse_idx[5]{$cur_row_ids[6]}[1]{tracks} }, $collapse_idx[6]{$cur_row_ids[6]}{$cur_row_ids[8]} ||= [{ title => $cur_row->[8] }]
-        unless $collapse_idx[6]{$cur_row_ids[6]}{$cur_row_ids[8]};
+      push @{ $collapse_idx[5]{$cur_row_ids[1]}{$cur_row_ids[6]}[1]{tracks} }, $collapse_idx[6]{$cur_row_ids[1]}{$cur_row_ids[6]}{$cur_row_ids[8]} ||= [{ title => $cur_row->[8] }]
+        unless $collapse_idx[6]{$cur_row_ids[1]}{$cur_row_ids[6]}{$cur_row_ids[8]};
 
-      $collapse_idx[1]{$cur_row_ids[1]}[1]{tracks} ||= [];
       push @{ $collapse_idx[1]{$cur_row_ids[1]}[1]{tracks} }, $collapse_idx[7]{$cur_row_ids[1]}{$cur_row_ids[5]} ||= [{ title => $cur_row->[5] }]
         unless $collapse_idx[7]{$cur_row_ids[1]}{$cur_row_ids[5]};
 
       $collapse_idx[7]{$cur_row_ids[1]}{$cur_row_ids[5]}[1]{lyrics} ||= $collapse_idx[8]{$cur_row_ids[1]}{$cur_row_ids[5] };
 
-      $collapse_idx[8]{$cur_row_ids[1]}{$cur_row_ids[5]}[1]{lyric_versions} ||= [];
       push @{ $collapse_idx[8]{$cur_row_ids[1]}{$cur_row_ids[5]}[1]{lyric_versions} }, $collapse_idx[9]{$cur_row_ids[0]}{$cur_row_ids[1]}{$cur_row_ids[5]} ||= [{ text => $cur_row->[0] }]
         unless $collapse_idx[9]{$cur_row_ids[0]}{$cur_row_ids[1]}{$cur_row_ids[5]};