X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=lib%2FDBIx%2FClass%2FResultSet.pm;h=a2e3a4cadefee218ea56f860fb07ac18896ab2ea;hb=1e4f9fb3b8bd1f54518bc2942554099356fa6524;hp=c3156abbca984526059bd261d5cfd1982b7c9425;hpb=459669a408682896779b908bc29d6264af8702e3;p=dbsrgits%2FDBIx-Class.git

diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index c3156ab..a2e3a4c 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -1077,8 +1077,8 @@ sub single {
     $attrs->{where}, $attrs
   )];
   return undef unless @$data;
-  $self->{stashed_rows} = [ $data ];
-  $self->_construct_objects->[0];
+  $self->{_stashed_rows} = [ $data ];
+  $self->_construct_results->[0];
 }
 
 
@@ -1247,39 +1247,47 @@ sub next {
     return ($self->all)[0];
   }
 
-  return shift(@{$self->{stashed_objects}}) if @{ $self->{stashed_objects}||[] };
+  return shift(@{$self->{_stashed_results}}) if @{ $self->{_stashed_results}||[] };
 
-  $self->{stashed_objects} = $self->_construct_objects
+  $self->{_stashed_results} = $self->_construct_results
     or return undef;
 
-  return shift @{$self->{stashed_objects}};
+  return shift @{$self->{_stashed_results}};
 }
 
-# Constructs as many objects as it can in one pass while respecting
+# Constructs as many results as it can in one pass while respecting
 # cursor laziness. Several modes of operation:
 #
-# * Always builds everything present in @{$self->{stashed_rows}}
+# * Always builds everything present in @{$self->{_stashed_rows}}
 # * If called with $fetch_all true - pulls everything off the cursor and
-#   builds all objects in one pass
+#   builds all result structures (or objects) in one pass
 # * If $self->_resolved_attrs->{collapse} is true, checks the order_by
 #   and if the resultset is ordered properly by the left side:
 #   * Fetches stuff off the cursor until the "master object" changes,
-#     and saves the last extra row (if any) in @{$self->{stashed_rows}}
+#     and saves the last extra row (if any) in @{$self->{_stashed_rows}}
 #   OR
 #   * Just fetches, and collapses/constructs everything as if $fetch_all
 #     was requested (there is no other way to collapse except for an
 #     eager cursor)
 # * If no collapse is requested - just get the next row, construct and
 #   return
-sub _construct_objects {
+sub _construct_results {
   my ($self, $fetch_all) = @_;
 
   my $rsrc = $self->result_source;
   my $attrs = $self->_resolved_attrs;
 
-  if (!$fetch_all and ! $attrs->{order_by} and $attrs->{collapse}) {
+  if (
+    ! $fetch_all
+      and
+    ! $attrs->{order_by}
+      and
+    $attrs->{collapse}
+      and
+    my @pcols = $rsrc->primary_columns
+  ) {
     # default order for collapsing unless the user asked for something
-    $attrs->{order_by} = [ map { join '.', $attrs->{alias}, $_} $rsrc->primary_columns ];
+    $attrs->{order_by} = [ map { join '.', $attrs->{alias}, $_} @pcols ];
     $attrs->{_ordered_for_collapse} = 1;
     $attrs->{_order_is_artificial} = 1;
   }
@@ -1287,9 +1295,11 @@ sub _construct_objects {
   my $cursor = $self->cursor;
 
   # this will be used as both initial raw-row collector AND as a RV of
-  # _construct_objects. Not regrowing the array twice matters a lot...
-  # a suprising amount actually
-  my $rows = delete $self->{stashed_rows};
+  # _construct_results. Not regrowing the array twice matters a lot...
+  # a surprising amount actually
+  my $rows = delete $self->{_stashed_rows};
+
+  my $did_fetch_all = $fetch_all;
 
   if ($fetch_all) {
     # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
@@ -1327,11 +1337,11 @@ sub _construct_objects {
     } unless defined $attrs->{_ordered_for_collapse};
 
     if (! $attrs->{_ordered_for_collapse}) {
-      $fetch_all = 1;
+      $did_fetch_all = 1;
 
       # instead of looping over ->next, use ->all in stealth mode
       # *without* calling a ->reset afterwards
-      # FIXME - encapsulation breach, got to be a better way
+      # FIXME ENCAPSULATION - encapsulation breach, cursor method additions pending
       if (! $cursor->{_done}) {
         $rows = [ ($rows ? @$rows : ()), $cursor->all ];
         $cursor->{_done} = 1;
@@ -1339,7 +1349,7 @@ sub _construct_objects {
     }
   }
 
-  if (! $fetch_all and ! @{$rows||[]} ) {
+  if (! $did_fetch_all and ! @{$rows||[]} ) {
     # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
     if (scalar (my @r = $cursor->next) ) {
       $rows = [ \@r ];
@@ -1349,12 +1359,12 @@ sub _construct_objects {
   return undef unless @{$rows||[]};
 
   my @extra_collapser_args;
-  if ($attrs->{collapse} and ! $fetch_all ) {
+  if ($attrs->{collapse} and ! $did_fetch_all ) {
 
     @extra_collapser_args = (
       # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
       sub { my @r = $cursor->next or return; \@r }, # how the collapser gets more rows
-      ($self->{stashed_rows} = []),                 # where does it stuff excess
+      ($self->{_stashed_rows} = []),                # where does it stuff excess
     );
   }
 
@@ -1368,14 +1378,25 @@ sub _construct_objects {
 
   my $infmap = $attrs->{as};
 
-  $self->{_result_inflator}{is_hri} = do { ( $inflator_cref == (
-    require DBIx::Class::ResultClass::HashRefInflator
-      &&
-    DBIx::Class::ResultClass::HashRefInflator->can('inflate_result')
-  ) ) ? 1 : 0
-  } unless defined $self->{_result_inflator}{is_hri};
 
-  if ($attrs->{_single_resultclass_inflation}) {
+  $self->{_result_inflator}{is_core_row} = ( (
+    $inflator_cref
+      ==
+    ( \&DBIx::Class::Row::inflate_result || die "No ::Row::inflate_result() - can't happen" )
+  ) ? 1 : 0 ) unless defined $self->{_result_inflator}{is_core_row};
+
+  $self->{_result_inflator}{is_hri} = ( (
+    ! $self->{_result_inflator}{is_core_row}
+      and
+    $inflator_cref == (
+      require DBIx::Class::ResultClass::HashRefInflator
+        &&
+      DBIx::Class::ResultClass::HashRefInflator->can('inflate_result')
+    )
+  ) ? 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 ($self->{_result_inflator}{is_hri}) {
       for my $r (@$rows) {
@@ -1401,7 +1422,7 @@ sub _construct_objects {
       );
     }
   }
-  # Special-case multi-object HRI (we always prune)
+  # Special-case multi-object HRI (we always prune, and there is no $inflator_cref pass)
   elsif ($self->{_result_inflator}{is_hri}) {
     ( $self->{_row_parser}{hri} ||= $rsrc->_mk_row_parser({
       eval => 1,
@@ -1410,26 +1431,34 @@ sub _construct_objects {
       collapse => $attrs->{collapse},
       premultiplied => $attrs->{_main_source_premultiplied},
       hri_style => 1,
+      prune_null_branches => 1,
     }) )->($rows, @extra_collapser_args);
   }
   # Regular multi-object
   else {
+    my $parser_type = $self->{_result_inflator}{is_core_row} ? 'classic_pruning' : 'classic_nonpruning';
 
-    ( $self->{_row_parser}{classic} ||= $rsrc->_mk_row_parser({
+    ( $self->{_row_parser}{$parser_type} ||= $rsrc->_mk_row_parser({
       eval => 1,
       inflate_map => $infmap,
       selection => $attrs->{select},
       collapse => $attrs->{collapse},
       premultiplied => $attrs->{_main_source_premultiplied},
+      prune_null_branches => $self->{_result_inflator}{is_core_row},
     }) )->($rows, @extra_collapser_args);
 
     $_ = $inflator_cref->($res_class, $rsrc, @$_) for @$rows;
   }
 
-  # CDBI compat stuff
-  if ($attrs->{record_filter}) {
-    $_ = $attrs->{record_filter}->($_) for @$rows;
-  }
+  # The @$rows check seems odd at first - why wouldn't we want to warn
+  # regardless? The issue is things like find() etc, where the user
+  # *knows* only one result will come back. In these cases the ->all
+  # is not a pessimization, but rather something we actually want
+  carp_unique(
+    'Unable to properly collapse has_many results in iterator mode due '
+  . 'to order criteria - performed an eager cursor slurp underneath. '
+  . 'Consider using ->all() instead'
+  ) if ( ! $fetch_all and @$rows > 1 );
 
   return $rows;
 }
@@ -1472,14 +1501,19 @@ sub result_class {
   my ($self, $result_class) = @_;
   if ($result_class) {
 
-    unless (ref $result_class) { # don't fire this for an object
-      $self->ensure_class_loaded($result_class);
+    # don't fire this for an object
+    $self->ensure_class_loaded($result_class)
+      unless ref($result_class);
+
+    if ($self->get_cache) {
+      carp_unique('Changing the result_class of a ResultSet instance with cached results is a noop - the cache contents will not be altered');
+    }
+    # FIXME ENCAPSULATION - encapsulation breach, cursor method additions pending
+    elsif ($self->{cursor} && $self->{cursor}{_pos}) {
+      $self->throw_exception('Changing the result_class of a ResultSet instance with an active cursor is not supported');
     }
+
     $self->_result_class($result_class);
-    # THIS LINE WOULD BE A BUG - this accessor specifically exists to
-    # permit the user to set result class on one result set only; it only
-    # chains if provided to search()
-    #$self->{attrs}{result_class} = $result_class if ref $self;
 
     delete $self->{_result_inflator};
   }
@@ -1577,7 +1611,7 @@ sub _count_rs {
 
   my $tmp_attrs = { %$attrs };
   # take off any limits, record_filter is cdbi, and no point of ordering nor locking a count
-  delete @{$tmp_attrs}{qw/rows offset order_by record_filter for/};
+  delete @{$tmp_attrs}{qw/rows offset order_by _related_results_construction record_filter for/};
 
   # overwrite the selector (supplied by the storage)
   $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $attrs);
@@ -1599,7 +1633,7 @@ sub _count_subq_rs {
 
   my $sub_attrs = { %$attrs };
   # extra selectors do not go in the subquery and there is no point of ordering it, nor locking it
-  delete @{$sub_attrs}{qw/collapse columns as select _prefetch_selector_range order_by for/};
+  delete @{$sub_attrs}{qw/collapse columns as select _related_results_construction order_by for/};
 
   # if we multi-prefetch we group_by something unique, as this is what we would
   # get out of the rs via ->next/->all. We *DO WANT* to clobber old group_by regardless
@@ -1727,7 +1761,7 @@ sub all {
     $self->throw_exception("all() doesn't take any arguments, you probably wanted ->search(...)->all()");
   }
 
-  delete @{$self}{qw/stashed_rows stashed_objects/};
+  delete @{$self}{qw/_stashed_rows _stashed_results/};
 
   if (my $c = $self->get_cache) {
     return @$c;
@@ -1735,7 +1769,7 @@ sub all {
 
   $self->cursor->reset;
 
-  my $objs = $self->_construct_objects('fetch_all') || [];
+  my $objs = $self->_construct_results('fetch_all') || [];
 
   $self->set_cache($objs) if $self->{attrs}{cache};
 
@@ -1761,7 +1795,7 @@ another query.
 sub reset {
   my ($self) = @_;
 
-  delete @{$self}{qw/stashed_rows stashed_objects/};
+  delete @{$self}{qw/_stashed_rows _stashed_results/};
   $self->{all_cache_position} = 0;
   $self->cursor->reset;
   return $self;
@@ -1802,7 +1836,7 @@ sub _rs_update_delete {
   my $attrs = { %{$self->_resolved_attrs} };
 
   my $join_classifications;
-  my $existing_group_by = delete $attrs->{group_by};
+  my ($existing_group_by) = delete @{$attrs}{qw(group_by _grouped_by_distinct)};
 
   # do we need a subquery for any reason?
   my $needs_subq = (
@@ -1863,7 +1897,7 @@ sub _rs_update_delete {
     );
 
     # make a new $rs selecting only the PKs (that's all we really need for the subq)
-    delete $attrs->{$_} for qw/collapse select _prefetch_selector_range as/;
+    delete $attrs->{$_} for qw/select as collapse _related_results_construction/;
     $attrs->{columns} = [ map { "$attrs->{alias}.$_" } @$idcols ];
     $attrs->{group_by} = \ '';  # FIXME - this is an evil hack, it causes the optimiser to kick in and throw away the LEFT joins
     my $subrs = (ref $self)->new($rsrc, $attrs);
@@ -3248,7 +3282,7 @@ sub _chain_relationship {
   # ->_resolve_join as otherwise they get lost - captainL
   my $join = $self->_merge_joinpref_attr( $attrs->{join}, $attrs->{prefetch} );
 
-  delete @{$attrs}{qw/join prefetch collapse group_by distinct select as columns +select +as +columns/};
+  delete @{$attrs}{qw/join prefetch collapse group_by distinct _grouped_by_distinct select as columns +select +as +columns/};
 
   my $seen = { %{ (delete $attrs->{seen_join}) || {} } };
 
@@ -3333,40 +3367,6 @@ sub _chain_relationship {
   return {%$attrs, from => $from, seen_join => $seen};
 }
 
-# FIXME - this needs to go live in Schema with the tree walker... or
-# something
-my $inflatemap_checker;
-$inflatemap_checker = sub {
-  my ($rsrc, $relpaths) = @_;
-
-  my $rels;
-
-  for (@$relpaths) {
-    $_ =~ /^ ( [^\.]+ ) \. (.+) $/x
-      or next;
-
-    push @{$rels->{$1}}, $2;
-  }
-
-  for my $rel (keys %$rels) {
-    my $rel_rsrc = try {
-      $rsrc->related_source ($rel)
-    } catch {
-    $rsrc->throw_exception(sprintf(
-      "Inflation into non-existent relationship '%s' of '%s' requested, "
-    . "check the inflation specification (columns/as) ending in '...%s.%s'",
-      $rel,
-      $rsrc->source_name,
-      $rel,
-      ( sort { length($a) <=> length ($b) } @{$rels->{$rel}} )[0],
-    ))};
-
-    $inflatemap_checker->($rel_rsrc, $rels->{$rel});
-  }
-
-  return;
-};
-
 sub _resolved_attrs {
   my $self = shift;
   return $self->{_attrs} if $self->{_attrs};
@@ -3438,14 +3438,6 @@ sub _resolved_attrs {
     }
   }
 
-  # validate the user-supplied 'as' chain
-  # folks get too confused by the (logical) exception message, need to
-  # go to some lengths to clarify the text
-  #
-  # FIXME - this needs to go live in Schema with the tree walker... or
-  # something
-  $inflatemap_checker->($source, \@as);
-
   $attrs->{select} = \@sel;
   $attrs->{as} = \@as;
 
@@ -3500,6 +3492,7 @@ sub _resolved_attrs {
       carp_unique ("Useless use of distinct on a grouped resultset ('distinct' is ignored when a 'group_by' is present)");
     }
     else {
+      $attrs->{_grouped_by_distinct} = 1;
       # distinct affects only the main selection part, not what prefetch may
       # add below.
       $attrs->{group_by} = $source->storage->_group_over_selection (
@@ -3545,18 +3538,14 @@ sub _resolved_attrs {
 
     my @prefetch = $source->_resolve_prefetch( $prefetch, $alias, $join_map );
 
-    # we need to somehow mark which columns came from prefetch
-    if (@prefetch) {
-      my $sel_end = $#{$attrs->{select}};
-      $attrs->{_prefetch_selector_range} = [ $sel_end + 1, $sel_end + @prefetch ];
-    }
-
     push @{ $attrs->{select} }, (map { $_->[0] } @prefetch);
     push @{ $attrs->{as} }, (map { $_->[1] } @prefetch);
   }
 
-  if ( ! List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) {
-    $attrs->{_single_resultclass_inflation} = 1;
+  if ( List::Util::first { $_ =~ /\./ } @{$attrs->{as}} ) {
+    $attrs->{_related_results_construction} = 1;
+  }
+  else {
     $attrs->{collapse} = 0;
   }