Introduce GOVERNANCE document and empty RESOLUTIONS file.
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSet.pm
index bf7e88f..030f292 100644 (file)
@@ -9,11 +9,13 @@ use DBIx::Class::Carp;
 use DBIx::Class::ResultSetColumn;
 use DBIx::Class::ResultClass::HashRefInflator;
 use Scalar::Util qw( blessed reftype );
+use SQL::Abstract 'is_literal_value';
 use DBIx::Class::_Util qw(
   dbic_internal_try dbic_internal_catch dump_value emit_loud_diag
-  fail_on_internal_wantarray fail_on_internal_call UNRESOLVABLE_CONDITION
+  fail_on_internal_call UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR
 );
 use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
+use DBIx::Class::ResultSource::FromSpec::Util 'find_join_path_to_alias';
 
 BEGIN {
   # De-duplication in _merge_attr() is disabled, but left in for reference
@@ -388,28 +390,27 @@ L<DBIx::Class::Manual::Cookbook/Formatting DateTime objects in queries>.
 
 =cut
 
-sub search {
-  my $self = shift;
-  my $rs = $self->search_rs( @_ );
+sub search :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
 
-  if (wantarray) {
-    DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_WANTARRAY and my $sog = fail_on_internal_wantarray;
-    return $rs->all;
-  }
-  elsif (defined wantarray) {
-    return $rs;
-  }
-  else {
-    # we can be called by a relationship helper, which in
-    # turn may be called in void context due to some braindead
-    # overload or whatever else the user decided to be clever
-    # at this particular day. Thus limit the exception to
-    # external code calls only
-    $self->throw_exception ('->search is *not* a mutator, calling it in void context makes no sense')
-      if (caller)[0] !~ /^\QDBIx::Class::/;
-
-    return ();
-  }
+  my $rs = shift->search_rs( @_ );
+
+  return $rs->all
+    if wantarray;
+
+  return $rs
+    if defined wantarray;
+
+  # we can be called by a relationship helper, which in
+  # turn may be called in void context due to some braindead
+  # overload or whatever else the user decided to be clever
+  # at this particular day. Thus limit the exception to
+  # external code calls only
+  $rs->throw_exception ('->search is *not* a mutator, calling it in void context makes no sense')
+    if (caller)[0] !~ /^\QDBIx::Class::/;
+
+  # we are in void ctx here, but just in case
+  return ();
 }
 
 =head2 search_rs
@@ -696,7 +697,9 @@ Example of how to use C<search> instead of C<search_literal>
 
 =cut
 
-sub search_literal {
+sub search_literal :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
   my ($self, $sql, @bind) = @_;
   my $attr;
   if ( @bind && ref($bind[-1]) eq 'HASH' ) {
@@ -776,7 +779,6 @@ sub find {
   my $self = shift;
   my $attrs = (@_ > 1 && ref $_[-1] eq 'HASH' ? pop(@_) : {});
 
-  my $rsrc = $self->result_source;
 
   my $constraint_name;
   if (exists $attrs->{key}) {
@@ -789,6 +791,8 @@ sub find {
   # Parse out the condition from input
   my $call_cond;
 
+  my $rsrc = $self->result_source;
+
   if (ref $_[0] eq 'HASH') {
     $call_cond = { %{$_[0]} };
   }
@@ -811,25 +815,59 @@ sub find {
   }
 
   # process relationship data if any
+  my $rel_list;
+
   for my $key (keys %$call_cond) {
     if (
+      # either a structure or a result-ish object
       length ref($call_cond->{$key})
         and
-      my $relinfo = $rsrc->relationship_info($key)
+      ( $rel_list ||= { map { $_ => 1 } $rsrc->relationships } )
+        ->{$key}
         and
-      # implicitly skip has_many's (likely MC)
-      ( ref( my $val = delete $call_cond->{$key} ) ne 'ARRAY' )
+      ! is_literal_value( $call_cond->{$key} )
+        and
+      # implicitly skip has_many's (likely MC), via the delete()
+      ( ref( my $foreign_val = delete $call_cond->{$key} ) ne 'ARRAY' )
     ) {
-      my ($rel_cond, $crosstable) = $rsrc->_resolve_condition(
-        $relinfo->{cond}, $val, $key, $key
-      );
 
-      $self->throw_exception("Complex condition via relationship '$key' is unsupported in find()")
-         if $crosstable or ref($rel_cond) ne 'HASH';
+      # FIXME: it seems wrong that relationship conditions take precedence...?
+      $call_cond = {
+        %$call_cond,
+
+        %{ $rsrc->resolve_relationship_condition(
+          require_join_free_values => 1,
+          rel_name => $key,
+          foreign_values => (
+            (! defined blessed $foreign_val) ? $foreign_val : do {
+
+              my $f_result_class = $rsrc->related_source($key)->result_class;
+
+              unless( $foreign_val->isa($f_result_class) ) {
+
+                $self->throw_exception(
+                  'Objects supplied to find() must inherit from '
+                . "'$DBIx::Class::ResultSource::__expected_result_class_isa'"
+                ) unless $foreign_val->isa(
+                  $DBIx::Class::ResultSource::__expected_result_class_isa
+                );
+
+                carp_unique(
+                  "Objects supplied to find() via '$key' usually should inherit from "
+                . "the related ResultClass ('$f_result_class'), perhaps you've made "
+                . 'a mistake?'
+                );
+              }
+
+              +{ $foreign_val->get_columns };
+            }
+          ),
 
-      # supplement condition
-      # relationship conditions take precedence (?)
-      @{$call_cond}{keys %$rel_cond} = values %$rel_cond;
+          # an API where these are optional would be too cumbersome,
+          # instead always pass in some dummy values
+          DUMMY_ALIASPAIR,
+        )->{join_free_values} },
+      };
     }
   }
 
@@ -896,7 +934,7 @@ sub find {
   }
 
   # Run the query, passing the result_class since it should propagate for find
-  my $rs = $self->search ($final_cond, {result_class => $self->result_class, %$attrs});
+  my $rs = $self->search_rs( $final_cond, {result_class => $self->result_class, %$attrs} );
   if ($rs->_resolved_attrs->{collapse}) {
     my $row = $rs->next;
     carp "Query returned more than one row" if $rs->next;
@@ -1154,7 +1192,9 @@ instead. An example conversion is:
 
 =cut
 
-sub search_like {
+sub search_like :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
   my $class = shift;
   carp_unique (
     'search_like() is deprecated and will be removed in DBIC version 0.09.'
@@ -1185,7 +1225,9 @@ three records, call:
 
 =cut
 
-sub slice {
+sub slice :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
   my ($self, $min, $max) = @_;
   my $attrs = {}; # = { %{ $self->{attrs} || {} } };
   $attrs->{offset} = $self->{attrs}{offset} || 0;
@@ -1569,7 +1611,7 @@ C<< $rs->search ($cond, \%attrs)->count >>
 
 sub count {
   my $self = shift;
-  return $self->search(@_)->count if @_ and defined $_[0];
+  return $self->search_rs(@_)->count if @_ and defined $_[0];
   return scalar @{ $self->get_cache } if $self->get_cache;
 
   my $attrs = { %{ $self->_resolved_attrs } };
@@ -1617,7 +1659,7 @@ the same single value obtainable via L</count>.
 
 sub count_rs {
   my $self = shift;
-  return $self->search(@_)->count_rs if @_;
+  return $self->search_rs(@_)->count_rs if @_;
 
   # this may look like a lack of abstraction (count() does about the same)
   # but in fact an _rs *must* use a subquery for the limits, as the
@@ -1744,7 +1786,7 @@ sub _count_subq_rs {
   return $rsrc->resultset_class
                ->new ($rsrc, $sub_attrs)
                 ->as_subselect_rs
-                 ->search ({}, { columns => { count => $rsrc->schema->storage->_count_select ($rsrc, $attrs) } })
+                 ->search_rs ({}, { columns => { count => $rsrc->schema->storage->_count_select ($rsrc, $attrs) } })
                   ->get_column ('count');
 }
 
@@ -1975,7 +2017,7 @@ sub _rs_update_delete {
           }
         }
 
-        $subrs = $subrs->search({}, { group_by => $attrs->{columns} });
+        $subrs = $subrs->search_rs({}, { group_by => $attrs->{columns} });
       }
 
       $guard = $storage->txn_scope_guard;
@@ -2220,6 +2262,7 @@ sub populate {
   # At this point assume either hashes or arrays
 
   my $rsrc = $self->result_source;
+  my $storage = $rsrc->schema->storage;
 
   if(defined wantarray) {
     my (@results, $guard);
@@ -2228,7 +2271,7 @@ sub populate {
       # column names only, nothing to do
       return if @$data == 1;
 
-      $guard = $rsrc->schema->storage->txn_scope_guard
+      $guard = $storage->txn_scope_guard
         if @$data > 2;
 
       @results = map
@@ -2238,7 +2281,7 @@ sub populate {
     }
     else {
 
-      $guard = $rsrc->schema->storage->txn_scope_guard
+      $guard = $storage->txn_scope_guard
         if @$data > 1;
 
       @results = map { $self->new_result($_)->insert } @$data;
@@ -2452,13 +2495,13 @@ sub populate {
 
 ### start work
   my $guard;
-  $guard = $rsrc->schema->storage->txn_scope_guard
+  $guard = $storage->txn_scope_guard
     if $slices_with_rels;
 
 ### main source data
   # FIXME - need to switch entirely to a coderef-based thing,
   # so that large sets aren't copied several times... I think
-  $rsrc->schema->storage->_insert_bulk(
+  $storage->_insert_bulk(
     $rsrc,
     [ @$colnames, sort keys %$rs_data ],
     [ map {
@@ -2492,18 +2535,20 @@ sub populate {
 
           $colinfo->{$rel}{rs} = $rsrc->related_source($rel)->resultset;
 
-          $colinfo->{$rel}{fk_map} = { reverse %{ $rsrc->_resolve_relationship_condition(
+          $colinfo->{$rel}{fk_map} = { reverse %{ $rsrc->resolve_relationship_condition(
             rel_name => $rel,
-            self_alias => "\xFE", # irrelevant
-            foreign_alias => "\xFF", # irrelevant
+
+            # an API where these are optional would be too cumbersome,
+            # instead always pass in some dummy values
+            DUMMY_ALIASPAIR,
           )->{identity_map} || {} } };
 
         }
 
-        $colinfo->{$rel}{rs}->search({ map # only so that we inherit them values properly, no actual search
+        $colinfo->{$rel}{rs}->search_rs({ map # only so that we inherit them values properly, no actual search
           {
             $_ => { '=' =>
-              ( $main_proto_rs ||= $rsrc->resultset->search($main_proto) )
+              ( $main_proto_rs ||= $rsrc->resultset->search_rs($main_proto) )
                 ->get_column( $colinfo->{$rel}{fk_map}{$_} )
                  ->as_query
             }
@@ -3269,13 +3314,11 @@ sub related_resultset {
 
     my $attrs = $self->_chain_relationship($rel);
 
-    my $storage = $rsrc->schema->storage;
-
     # Previously this atribute was deleted (instead of being set as it is now)
     # Doing so seems to be harmless in all available test permutations
     # See also 01d59a6a6 and mst's comment below
     #
-    $attrs->{alias} = $storage->relname_to_table_alias(
+    $attrs->{alias} = $rsrc->schema->storage->relname_to_table_alias(
       $rel,
       $attrs->{seen_join}{$rel}
     );
@@ -3283,8 +3326,55 @@ sub related_resultset {
     # since this is search_related, and we already slid the select window inwards
     # (the select/as attrs were deleted in the beginning), we need to flip all
     # left joins to inner, so we get the expected results
-    # read the comment on top of the actual function to see what this does
-    $attrs->{from} = $storage->_inner_join_to_node( $attrs->{from}, $attrs->{alias} );
+    #
+    # The DBIC relationship chaining implementation is pretty simple - every
+    # new related_relationship is pushed onto the {from} stack, and the {select}
+    # window simply slides further in. This means that when we count somewhere
+    # in the middle, we got to make sure that everything in the join chain is an
+    # actual inner join, otherwise the count will come back with unpredictable
+    # results (a resultset may be generated with _some_ rows regardless of if
+    # the relation which the $rs currently selects has rows or not). E.g.
+    # $artist_rs->cds->count - normally generates:
+    # SELECT COUNT( * ) FROM artist me LEFT JOIN cd cds ON cds.artist = me.artistid
+    # which actually returns the number of artists * (number of cds || 1)
+    #
+    # So what we do here is crawl {from}, determine if the current alias is at
+    # the top of the stack, and if not - make sure the chain is inner-joined down
+    # to the root.
+    #
+    my $switch_branch = find_join_path_to_alias(
+      $attrs->{from},
+      $attrs->{alias},
+    );
+
+    if ( @{ $switch_branch || [] } ) {
+
+      # So it looks like we will have to switch some stuff around.
+      # local() is useless here as we will be leaving the scope
+      # anyway, and deep cloning is just too fucking expensive
+      # So replace the first hashref in the node arrayref manually
+      my @new_from = $attrs->{from}[0];
+      my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path
+
+      for my $j ( @{$attrs->{from}}[ 1 .. $#{$attrs->{from}} ] ) {
+        my $jalias = $j->[0]{-alias};
+
+        if ($sw_idx->{$jalias}) {
+          my %attrs = %{$j->[0]};
+          delete $attrs{-join_type};
+          push @new_from, [
+            \%attrs,
+            @{$j}[ 1 .. $#$j ],
+          ];
+        }
+        else {
+          push @new_from, $j;
+        }
+      }
+
+      $attrs->{from} = \@new_from;
+    }
+
 
     #XXX - temp fix for result_class bug. There likely is a more elegant fix -groditi
     delete $attrs->{result_class};
@@ -3418,9 +3508,19 @@ but because we isolated the group by into a subselect the above works.
 =cut
 
 sub as_subselect_rs {
+
+  # FIXME - remove at some point in the future (2018-ish)
+  wantarray
+    and
+  carp_unique(
+    'Starting with DBIC@0.082900 as_subselect_rs() always returns a ResultSet '
+  . 'instance regardless of calling context. Please force scalar() context to '
+  . 'silence this warning'
+  );
+
   my $self = shift;
 
-  my $attrs = $self->_resolved_attrs;
+  my $alias = $self->current_source_alias;
 
   my $fresh_rs = (ref $self)->new (
     $self->result_source
@@ -3430,13 +3530,13 @@ sub as_subselect_rs {
   delete $fresh_rs->{cond};
   delete @{$fresh_rs->{attrs}}{qw/where bind/};
 
-  return $fresh_rs->search( {}, {
+  $fresh_rs->search_rs( {}, {
     from => [{
-      $attrs->{alias} => $self->as_query,
-      -alias  => $attrs->{alias},
+      $alias => $self->as_query,
+      -alias  => $alias,
       -rsrc   => $self->result_source,
     }],
-    alias => $attrs->{alias},
+    alias => $alias,
   });
 }
 
@@ -3477,7 +3577,7 @@ sub _chain_relationship {
     # Nuke the prefetch (if any) before the new $rs attrs
     # are resolved (prefetch is useless - we are wrapping
     # a subquery anyway).
-    my $rs_copy = $self->search;
+    my $rs_copy = $self->search_rs;
     $rs_copy->{attrs}{join} = $self->_merge_joinpref_attr (
       $rs_copy->{attrs}{join},
       delete $rs_copy->{attrs}{prefetch},