Merge branch 'topic/constructor_rewrite'
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSet.pm
index 2c0207e..d4c271a 100644 (file)
@@ -1237,14 +1237,22 @@ sub next {
   return shift @{$self->{stashed_objects}};
 }
 
-# takes a single DBI-row of data and coinstructs as many objects
-# as the resultset attributes call for.
-# This can be a bit of an action at a distance - it takes as an argument
-# the *current* cursor-row (already taken off the $sth), but if
-# collapsing is requested it will keep advancing the cursor either
-# until the current row-object is assembled (the collapser was able to
-# order the result sensibly) OR until the cursor is exhausted (an
-# unordered collapsing resultset effectively triggers ->all)
+# Constructs as many objects as it can in one pass while respecting
+# cursor laziness. Several modes of operation:
+#
+# * 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
+# * 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}}
+#   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 {
   my ($self, $fetch_all) = @_;
 
@@ -1257,10 +1265,11 @@ sub _construct_objects {
   # a suprising amount actually
   my $rows = (delete $self->{stashed_rows}) || [];
   if ($fetch_all) {
-    # FIXME - we can do better, cursor->next/all (well diff. methods) should return a ref
+    # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
     $rows = [ @$rows, $cursor->all ];
   }
   elsif (!$attrs->{collapse}) {
+    # FIXME SUBOPTIMAL - we can do better, cursor->next/all (well diff. methods) should return a ref
     push @$rows, do { my @r = $cursor->next; @r ? \@r : () }
       unless @$rows;
   }
@@ -1286,8 +1295,8 @@ sub _construct_objects {
       }
 
       # since all we check here are the start of the order_by belonging to the
-      # top level $rsrc, the order stability check will fail unless the whole
-      # thing is ordered as we need it
+      # top level $rsrc, a present identifying set will mean that the resultset
+      # is ordered by its leftmost table in a tsable manner
       (@ord_cols and $rsrc->_identifying_column_set({ map
         { $colinfos->{$_}{-colname} => $colinfos->{$_} }
         @ord_cols
@@ -1298,6 +1307,7 @@ sub _construct_objects {
       push @$rows, do { my @r = $cursor->next; @r ? \@r : () };
     }
     # instead of looping over ->next, use ->all in stealth mode
+    # FIXME - encapsulation breach, got to be a better way
     elsif (! $cursor->{done}) {
       push @$rows, $cursor->all;
       $cursor->{done} = 1;
@@ -1342,8 +1352,9 @@ sub _construct_objects {
       selection => $attrs->{select},
       collapse => $attrs->{collapse},
     }) or die $@)->($rows, $fetch_all ? () : (
-      sub { my @r = $cursor->next or return; \@r },
-      ($self->{stashed_rows} = []),
+      # 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
     ));  # modify $rows in-place, shrinking/extending as necessary
 
     $_ = $inflator->($res_class, $rsrc, @$_) for @$rows;
@@ -1432,8 +1443,7 @@ sub count {
 
   # this is a little optimization - it is faster to do the limit
   # adjustments in software, instead of a subquery
-  my $rows = delete $attrs->{rows};
-  my $offset = delete $attrs->{offset};
+  my ($rows, $offset) = delete @{$attrs}{qw/rows offset/};
 
   my $crs;
   if ($self->_has_resolved_attr (qw/collapse group_by/)) {
@@ -1504,7 +1514,6 @@ sub _count_rs {
   # overwrite the selector (supplied by the storage)
   $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $attrs);
   $tmp_attrs->{as} = 'count';
-  delete @{$tmp_attrs}{qw/columns/};
 
   my $tmp_rs = $rsrc->resultset_class->new($rsrc, $tmp_attrs)->get_column ('count');
 
@@ -1646,8 +1655,7 @@ sub all {
     $self->throw_exception("all() doesn't take any arguments, you probably wanted ->search(...)->all()");
   }
 
-  delete $self->{stashed_rows};
-  delete $self->{stashed_objects};
+  delete @{$self}{qw/stashed_rows stashed_objects/};
 
   if (my $c = $self->get_cache) {
     return @$c;
@@ -1680,9 +1688,8 @@ another query.
 
 sub reset {
   my ($self) = @_;
-  delete $self->{_attrs};
-  delete $self->{stashed_rows};
-  delete $self->{stashed_objects};
+
+  delete @{$self}{qw/_attrs stashed_rows stashed_objects/};
 
   $self->{all_cache_position} = 0;
   $self->cursor->reset;
@@ -1786,7 +1793,7 @@ sub _rs_update_delete {
   my $existing_group_by = delete $attrs->{group_by};
 
   # 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}{qw/collapse select _prefetch_selector_range as/};
   $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);
@@ -2233,7 +2240,7 @@ sub pager {
   # throw away the paging flags and re-run the count (possibly
   # with a subselect) to get the real total count
   my $count_attrs = { %$attrs };
-  delete $count_attrs->{$_} for qw/rows offset page pager/;
+  delete @{$count_attrs}{qw/rows offset page pager/};
 
   my $total_rs = (ref $self)->new($self->result_source, $count_attrs);
 
@@ -3705,10 +3712,8 @@ sub STORABLE_freeze {
   my $to_serialize = { %$self };
 
   # A cursor in progress can't be serialized (and would make little sense anyway)
-  delete $to_serialize->{cursor};
-
-  # the parser can be regenerated
-  delete $to_serialize->{_row_parser};
+  # the parser can be regenerated (and can't be serialized)
+  delete @{$to_serialize}{qw/cursor _row_parser/};
 
   # nor is it sensical to store a not-yet-fired-count pager
   if ($to_serialize->{pager} and ref $to_serialize->{pager}{total_entries} eq 'CODE') {