From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 27 Jul 2014 12:13:45 +0000 (+0200)
Subject: Ensure collapse is respected regardless of selection type
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=6aa939284368cb14aff81ab3c5e3945527f949b0;p=dbsrgits%2FDBIx-Class-Historic.git

Ensure collapse is respected regardless of selection type

Switch the attribute name/logic (passthrough on explicit presence only)
---

diff --git a/Changes b/Changes
index 9932b08..2aebd6d 100644
--- a/Changes
+++ b/Changes
@@ -21,6 +21,7 @@ Revision history for DBIx::Class
         - Fix on_connect_* not always firing in some cases - a race condition
           existed between storage accessor setters and the determine_driver
           routines, triggering a connection before the set-cycle is finished
+        - Fix collapse being ignored on single-origin selection (RT#95658)
         - Fix failure to detect stable order criteria when in iterator
           mode of a has_many prefetch off a search_related chain
         - Prevent erroneous database hit when accessing prefetched related
diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index b703c8f..44adfe8 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -312,7 +312,7 @@ sub new {
     if $source->isa('DBIx::Class::ResultSourceHandle');
 
   $attrs = { %{$attrs||{}} };
-  delete @{$attrs}{qw(_last_sqlmaker_alias_map _related_results_construction)};
+  delete @{$attrs}{qw(_last_sqlmaker_alias_map _simple_passthrough_construction)};
 
   if ($attrs->{page}) {
     $attrs->{rows} ||= 10;
@@ -1415,8 +1415,8 @@ sub _construct_results {
   ) ? 1 : 0 ) unless defined $self->{_result_inflator}{is_hri};
 
 
-  if (! $attrs->{_related_results_construction}) {
-    # construct a much simpler array->hash folder for the one-table cases right here
+  if ($attrs->{_simple_passthrough_construction}) {
+    # construct a much simpler array->hash folder for the one-table HRI cases right here
     if ($self->{_result_inflator}{is_hri}) {
       for my $r (@$rows) {
         $r = { map { $infmap->[$_] => $r->[$_] } 0..$#$infmap };
@@ -3749,10 +3749,11 @@ sub _resolved_attrs {
   push @{$attrs->{select}}, @prefetch_select;
   push @{$attrs->{as}}, @prefetch_as;
 
-  # whether we can get away with the dumbest (possibly DBI-internal) collapser
-  if ( List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) {
-    $attrs->{_related_results_construction} = 1;
-  }
+  $attrs->{_simple_passthrough_construction} = !(
+    $attrs->{collapse}
+      or
+    grep { $_ =~ /\./ } @{$attrs->{as}}
+  );
 
   # if both page and offset are specified, produce a combined offset
   # even though it doesn't make much sense, this is what pre 081xx has
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 18dbbb9..139dc49 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -2469,7 +2469,7 @@ sub _select_args {
     # are happy (this includes MySQL in strict_mode)
     # If any of the other joined tables are referenced in the group_by
     # however - the user is on their own
-    ( $prefetch_needs_subquery or $attrs->{_related_results_construction} )
+    ( $prefetch_needs_subquery or ! $attrs->{_simple_passthrough_construction} )
       and
     $attrs->{group_by}
       and
diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 26f8dca..c7910be 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -111,8 +111,8 @@ sub _adjust_select_args_for_complex_prefetch {
   my $outer_attrs = { %$attrs };
   delete @{$outer_attrs}{qw(from bind rows offset group_by _grouped_by_distinct having)};
 
-  my $inner_attrs = { %$attrs };
-  delete @{$inner_attrs}{qw(for collapse select as _related_results_construction)};
+  my $inner_attrs = { %$attrs, _simple_passthrough_construction => 1 };
+  delete @{$inner_attrs}{qw(for collapse select as)};
 
   # there is no point of ordering the insides if there is no limit
   delete $inner_attrs->{order_by} if (
diff --git a/t/count/joined.t b/t/count/joined.t
index 352eef9..bb8eb4c 100644
--- a/t/count/joined.t
+++ b/t/count/joined.t
@@ -7,24 +7,37 @@ use lib qw(t/lib);
 
 use DBICTest;
 
-plan tests => 7;
-
 my $schema = DBICTest->init_schema();
 
 my $cds = $schema->resultset("CD")->search({ cdid => 1 }, { join => { cd_to_producer => 'producer' } });
 cmp_ok($cds->count, '>', 1, "extra joins explode entity count");
 
-is (
-  $cds->search({}, { prefetch => 'cd_to_producer' })->count,
-  1,
-  "Count correct with extra joins collapsed by prefetch"
-);
-
-is (
-  $cds->search({}, { distinct => 1 })->count,
-  1,
-  "Count correct with requested distinct collapse of main table"
-);
+for my $arg (
+  [ 'prefetch-collapsed has_many' => { prefetch => 'cd_to_producer' } ],
+  [ 'distict-collapsed result' => { distinct => 1 } ],
+  [ 'explicit collapse request' => { collapse => 1 } ],
+) {
+  for my $hri (0,1) {
+    my $diag = $arg->[0] . ($hri ? ' with HRI' : '');
+
+    my $rs = $cds->search({}, {
+      %{$arg->[1]},
+      $hri ? ( result_class => 'DBIx::Class::ResultClass::HashRefInflator' ) : (),
+    });
+
+    is
+      $rs->count,
+      1,
+      "Count correct on $diag",
+    ;
+
+    is
+      scalar $rs->all,
+      1,
+      "Amount of constructed objects matches count on $diag",
+    ;
+  }
+}
 
 # JOIN and LEFT JOIN issues mean that we've seen problems where counted rows and fetched rows are sometimes 1 higher than they should
 # be in the related resultset.
@@ -35,3 +48,5 @@ is(scalar($artist->related_resultset('cds')->all()), 0, "No CDs fetched for a sh
 my $artist_rs = $schema->resultset('Artist')->search({artistid => $artist->id});
 is($artist_rs->related_resultset('cds')->count(), 0, "No CDs counted for a shiny new artist using a resultset search");
 is(scalar($artist_rs->related_resultset('cds')->all), 0, "No CDs fetched for a shiny new artist using a resultset search");
+
+done_testing;