Add notes about handling of inflated objects in resultset conditions
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSet.pm
index aabae51..36647ed 100644 (file)
@@ -12,6 +12,7 @@ use DBIx::Class::ResultSourceHandle;
 use List::Util ();
 use Scalar::Util qw/blessed weaken/;
 use Try::Tiny;
+use Storable qw/nfreeze thaw/;
 use namespace::clean;
 
 use overload
@@ -244,12 +245,32 @@ documentation for the first argument, see L<SQL::Abstract>.
 
 For more help on using joins with search, see L<DBIx::Class::Manual::Joining>.
 
+=head3 CAVEAT
+
+Note that L</search> does not process/deflate any of the values passed in the
+L<SQL::Abstract>-compatible search condition structure. This is unlike other
+condition-bound methods L</new>, L</create> and L</find>. The user must ensure
+manually that any value passed to this method will stringify to something the
+RDBMS knows how to deal with. A notable example is the handling of L<DateTime>
+objects, for more info see:
+L<DBIx::Class::Manual::Cookbook/Formatting_DateTime_objects_in_queries>.
+
 =cut
 
 sub search {
   my $self = shift;
   my $rs = $self->search_rs( @_ );
-  return (wantarray ? $rs->all : $rs);
+
+  my $want = wantarray;
+  if ($want) {
+    return $rs->all;
+  }
+  elsif (defined $want) {
+    return $rs;
+  }
+  else {
+    $self->throw_exception ('->search is *not* a mutator, calling it in void context makes no sense');
+  }
 }
 
 =head2 search_rs
@@ -276,7 +297,9 @@ sub search_rs {
   }
 
   my $call_attrs = {};
-  $call_attrs = pop(@_) if @_ > 1 and ref $_[-1] eq 'HASH';
+  $call_attrs = pop(@_) if (
+   @_ > 1 and ( ! defined $_[-1] or ref $_[-1] eq 'HASH' )
+  );
 
   # see if we can keep the cache (no $rs changes)
   my $cache;
@@ -326,6 +349,9 @@ sub search_rs {
 
   } if @_;
 
+  carp 'search( %condition ) is deprecated, use search( \%condition ) instead'
+    if (@_ > 1 and ! $self->result_source->result_class->isa('DBIx::Class::CDBICompat') );
+
   for ($old_where, $call_cond) {
     if (defined $_) {
       $new_attrs->{where} = $self->_stack_cond (
@@ -439,14 +465,14 @@ which are fully defined by the available condition.
 If no such constraint is found, C<find> currently defaults to a simple
 C<< search->(\%column_values) >> which may or may not do what you expect.
 Note that this fallback behavior may be deprecated in further versions. If
-you need to search with arbitrary conditions - use L</search>.
+you need to search with arbitrary conditions - use L</search>. If the query
+resulting from this fallback produces more than one row, a warning to the
+effect is issued, though only the first row is constructed and returned as
+C<$row_object>.
 
 In addition to C<key>, L</find> recognizes and applies standard
 L<resultset attributes|/ATTRIBUTES> in the same way as L</search> does.
 
-If the resulting query produces more than one row, a warning to the effect is
-issued, though only the first row is constructed and returned as C<$row_object>
-
 Note that if you have extra concerns about the correctness of the resulting
 query you need to specify the C<key> attribute and supply the entire condition
 as an argument to find (since it is not always possible to perform the
@@ -559,7 +585,7 @@ sub find {
 
     $final_cond = @unique_queries
       ? [ map { $self->_qualify_cond_columns($_, $alias) } @unique_queries ]
-      : $self->_qualify_cond_columns($call_cond, $alias)
+      : $self->_non_unique_find_fallback ($call_cond, $attrs)
     ;
   }
 
@@ -575,6 +601,30 @@ sub find {
   }
 }
 
+# This is a stop-gap method as agreed during the discussion on find() cleanup:
+# http://lists.scsys.co.uk/pipermail/dbix-class/2010-October/009535.html
+#
+# It is invoked when find() is called in legacy-mode with insufficiently-unique
+# condition. It is provided for overrides until a saner way forward is devised
+#
+# *NOTE* This is not a public method, and it's *GUARANTEED* to disappear down
+# the road. Please adjust your tests accordingly to catch this situation early
+# DBIx::Class::ResultSet->can('_non_unique_find_fallback') is reasonable
+#
+# The method will not be removed without an adequately complete replacement
+# for strict-mode enforcement
+sub _non_unique_find_fallback {
+  my ($self, $cond, $attrs) = @_;
+
+  return $self->_qualify_cond_columns(
+    $cond,
+    exists $attrs->{alias}
+      ? $attrs->{alias}
+      : $self->{attrs}{alias}
+  );
+}
+
+
 sub _qualify_cond_columns {
   my ($self, $cond, $alias) = @_;
 
@@ -603,7 +653,7 @@ sub _build_unique_cond {
   $final_cond = { map { $_ => $final_cond->{$_} } @c_cols };
 
   if (my @missing = grep { ! defined $final_cond->{$_} } (@c_cols) ) {
-    $self->throw_exception( sprintf ( "Unable to satisfy constraint '%s', no values for column(s): %s",
+    $self->throw_exception( sprintf ( "Unable to satisfy requested constraint '%s', no values for column(s): %s",
       $constraint_name,
       join (', ', map { "'$_'" } @missing),
     ) );
@@ -1471,6 +1521,15 @@ The return value is a pass through of what the underlying
 storage backend returned, and may vary. See L<DBI/execute> for the most
 common case.
 
+=head3 CAVEAT
+
+Note that L</update> does not process/deflate any of the values passed in.
+This is unlike the corresponding L<DBIx::Class::Row/update>. The user must
+ensure manually that any value passed to this method will stringify to
+something the RDBMS knows how to deal with. A notable example is the
+handling of L<DateTime> objects, for more info see:
+L<DBIx::Class::Manual::Cookbook/Formatting_DateTime_objects_in_queries>.
+
 =cut
 
 sub update {
@@ -1838,7 +1897,8 @@ my $mk_lazy_count_wizard = sub {
 
 # the tie class for 5.8.1
 {
-  package DBIx::Class::__DBIC_LAZY_RS_COUNT__;
+  package # hide from pause
+    DBIx::Class::__DBIC_LAZY_RS_COUNT__;
   use base qw/Tie::Hash/;
 
   sub FIRSTKEY { my $dummy = scalar keys %{$_[0]{data}}; each %{$_[0]{data}} }
@@ -3097,34 +3157,9 @@ sub _resolved_attrs {
       carp ("Useless use of distinct on a grouped resultset ('distinct' is ignored when a 'group_by' is present)");
     }
     else {
-      my $storage = $self->result_source->schema->storage;
-      my $rs_column_list = $storage->_resolve_column_info ($attrs->{from});
-
-      my $group_spec = $attrs->{group_by} = [];
-      my %group_index;
-
-      for (@{$attrs->{select}}) {
-        if (! ref($_) or ref ($_) ne 'HASH' ) {
-          push @$group_spec, $_;
-          $group_index{$_}++;
-          if ($rs_column_list->{$_} and $_ !~ /\./ ) {
-            # add a fully qualified version as well
-            $group_index{"$rs_column_list->{$_}{-source_alias}.$_"}++;
-          }
-        }
-      }
-      # add any order_by parts that are not already present in the group_by
-      # we need to be careful not to add any named functions/aggregates
-      # i.e. select => [ ... { count => 'foo', -as 'foocount' } ... ]
-      for my $chunk ($storage->_extract_order_columns($attrs->{order_by})) {
-
-        # only consider real columns (for functions the user got to do an explicit group_by)
-        my $colinfo = $rs_column_list->{$chunk}
-          or next;
-
-        $chunk = "$colinfo->{-source_alias}.$chunk" if $chunk !~ /\./;
-        push @$group_spec, $chunk unless $group_index{$chunk}++;
-      }
+      $attrs->{group_by} = $source->storage->_group_over_selection (
+        @{$attrs}{qw/from select order_by/}
+      );
     }
   }
 
@@ -3294,6 +3329,7 @@ sub _merge_attr {
   return $orig;
 }
 
+
 sub result_source {
     my $self = shift;
 
@@ -3304,6 +3340,27 @@ sub result_source {
     }
 }
 
+
+sub STORABLE_freeze {
+  my ($self, $cloning) = @_;
+  my $to_serialize = { %$self };
+
+  # A cursor in progress can't be serialized (and would make little sense anyway)
+  delete $to_serialize->{cursor};
+
+  return nfreeze($to_serialize);
+}
+
+# need this hook for symmetry
+sub STORABLE_thaw {
+  my ($self, $cloning, $serialized) = @_;
+
+  %$self = %{ thaw($serialized) };
+
+  return $self;
+}
+
+
 =head2 throw_exception
 
 See L<DBIx::Class::Schema/throw_exception> for details.