Merge the relationship resolution rework
Peter Rabbitson [Tue, 27 Sep 2016 12:19:39 +0000 (14:19 +0200)]
Done as a merge to aid bisecting IFF it ever becomes necessary

Wide testing indicates zero need for downstream changes (aside from several
warnings)

31 files changed:
.mailmap
AUTHORS
Changes
lib/DBIx/Class/Relationship/Accessor.pm
lib/DBIx/Class/Relationship/Base.pm
lib/DBIx/Class/Relationship/ManyToMany.pm
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSource.pm
lib/DBIx/Class/ResultSource/FromSpec/Util.pm [new file with mode: 0644]
lib/DBIx/Class/ResultSource/RowParser.pm
lib/DBIx/Class/Row.pm
lib/DBIx/Class/SQLMaker.pm
lib/DBIx/Class/SQLMaker/Util.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/ADO/MS_Jet.pm
lib/DBIx/Class/Storage/DBI/ADO/MS_Jet/Cursor.pm
lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server.pm
lib/DBIx/Class/Storage/DBI/ADO/Microsoft_SQL_Server/Cursor.pm
lib/DBIx/Class/Storage/DBI/SQLAnywhere.pm
lib/DBIx/Class/Storage/DBI/SQLAnywhere/Cursor.pm
lib/DBIx/Class/Storage/DBIHacks.pm
lib/DBIx/Class/_Util.pm
lib/SQL/Translator/Parser/DBIx/Class.pm
t/cdbi/06-hasa.t
t/cdbi/18-has_a.t
t/relationship/core.t
t/relationship/custom.t
t/sqlmaker/bind_transport.t
xt/extra/diagnostics/find_via_unsupported_rel.t [new file with mode: 0644]
xt/extra/diagnostics/invalid_resolve_relationship_condition_arguments.t [moved from t/relationship/resolve_relationship_condition.t with 58% similarity]
xt/extra/internals/namespaces_cleaned.t

index 3a45040..031804b 100644 (file)
--- a/.mailmap
+++ b/.mailmap
@@ -37,6 +37,7 @@ Jason M. Mills <jmmills@cpan.org>           <jmmills@cpan.org>
 Jonathan Chu <milki@rescomp.berkeley.edu>   <milki@rescomp.berkeley.edu>
 Jose Luis Martinez <jlmartinez@capside.com> <jlmartinez@capside.com>
 Kent Fredric <kentnl@cpan.org>              <kentfredric@gmail.com>
+Mark Zealey <mark@dns-consultants.com>      <mark@markandruth.co.uk>
 Matt Phillips <mattp@cpan.org>              <mphillips@oanda.com>
 Matt Phillips <mattp@cpan.org>              <matt@raybec.com>
 Michael Reddick <michael.reddick@gmail.com> <michaelr@michaelr-desktop.(none)>
diff --git a/AUTHORS b/AUTHORS
index 36e3991..9e4a962 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -97,6 +97,7 @@ ilmari: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
 ingy: Ingy döt Net <ingy@ingy.net>
 initself: Mike Baas <mike@initselftech.com>
 ironcamel: Naveed Massjouni <naveedm9@gmail.com>
+jalh: Mark Zealey <mark@dns-consultants.com>
 jasonmay: Jason May <jason.a.may@gmail.com>
 jawnsy: Jonathan Yu <jawnsy@cpan.org>
 jegade: Jens Gassmann <jens.gassmann@atomix.de>
diff --git a/Changes b/Changes
index 91c525b..abf2636 100644 (file)
--- a/Changes
+++ b/Changes
@@ -40,6 +40,9 @@ Revision history for DBIx::Class
           of Schema/Result/ResultSet classes loudly alerting the end user to
           potential extremely hard-to-diagnose pitfalls ( RT#93976, also fully
           addresses https://blog.afoolishmanifesto.com/posts/mros-and-you/ )
+        - A new low-level API for relationship resolution is available as an
+          official method ( $rsrc->resolve_relationship_condition ). This is
+          mainly of interest to builders of reflection tools
         - InflateColumn::DateTime now accepts the ecosystem-standard option
           'time_zone', in addition to the DBIC-only 'timezone' (GH#28)
         - Massively optimised literal SQL snippet scanner - fixes all known
@@ -68,6 +71,8 @@ Revision history for DBIx::Class
           create()/populate()
         - Fix spurious warning on MSSQL cursor invalidation retries (RT#102166)
         - Fix incorrect ::Storage->_ping() behavior under Sybase (RT#114214)
+        - Fix some corner cases of non-fatal failures during relationship
+          resolution showing up as hard errors
         - Fix several corner cases with Many2Many over custom relationships
         - Fix intermittent failure to infer the CASCADE attributes of relations
           during deployment_statements()/deploy()
index 8fdeab2..e6d4fb4 100644 (file)
@@ -46,21 +46,24 @@ sub add_relationship_accessor {
       else {
         my $rsrc = $self->result_source;
 
-        my $relcond = $rsrc->_resolve_relationship_condition(
-          rel_name => %1$s,
-          foreign_alias => %1$s,
-          self_alias => 'me',
-          self_result_object => $self,
-        );
+        my $jfc;
 
         return undef if (
-          $relcond->{join_free_condition}
-            and
-          $relcond->{join_free_condition} ne DBIx::Class::_Util::UNRESOLVABLE_CONDITION
+
+          $rsrc->relationship_info(%1$s)->{attrs}{undef_on_null_fk}
+
             and
-          scalar grep { not defined $_ } values %%{ $relcond->{join_free_condition} || {} }
+
+          $jfc = ( $rsrc->resolve_relationship_condition(
+            rel_name => %1$s,
+            foreign_alias => %1$s,
+            self_alias => 'me',
+            self_result_object => $self,
+          )->{join_free_condition} || {} )
+
             and
-          $rsrc->relationship_info(%1$s)->{attrs}{undef_on_null_fk}
+
+          grep { not defined $_ } values %%$jfc
         );
 
         my $val = $self->related_resultset( %1$s )->single;
index 8e4b280..5924db0 100644 (file)
@@ -6,7 +6,18 @@ use warnings;
 use base qw/DBIx::Class/;
 
 use Scalar::Util qw/weaken blessed/;
-use DBIx::Class::_Util qw( UNRESOLVABLE_CONDITION fail_on_internal_call );
+use DBIx::Class::_Util qw(
+  UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR
+  dbic_internal_try dbic_internal_catch fail_on_internal_call
+);
+use DBIx::Class::SQLMaker::Util 'extract_equality_conditions';
+use DBIx::Class::Carp;
+
+# FIXME - this should go away
+# instead Carp::Skip should export usable keywords or something like that
+my $unique_carper;
+BEGIN { $unique_carper = \&carp_unique }
+
 use namespace::clean;
 
 =head1 NAME
@@ -513,83 +524,119 @@ sub related_resultset {
 
   my ($self, $rel) = @_;
 
-  return $self->{related_resultsets}{$rel} = do {
+  my $rsrc = $self->result_source;
 
-    my $rsrc = $self->result_source;
+  my $rel_info = $rsrc->relationship_info($rel)
+    or $self->throw_exception( "No such relationship '$rel'" );
 
-    my $rel_info = $rsrc->relationship_info($rel)
-      or $self->throw_exception( "No such relationship '$rel'" );
+  my $relcond_is_freeform = ref $rel_info->{cond} eq 'CODE';
 
-    my $cond_res = $rsrc->_resolve_relationship_condition(
-      rel_name => $rel,
-      self_result_object => $self,
+  my $rrc_args = {
+    rel_name => $rel,
+    self_result_object => $self,
 
-      # this may look weird, but remember that we are making a resultset
-      # out of an existing object, with the new source being at the head
-      # of the FROM chain. Having a 'me' alias is nothing but expected there
-      foreign_alias => 'me',
+    # an extra sanity check guard
+    require_join_free_condition => !!(
+      ! $relcond_is_freeform
+        and
+      $self->in_storage
+    ),
 
-      self_alias => "!!!\xFF()!!!_SHOULD_NEVER_BE_SEEN_IN_USE_!!!()\xFF!!!",
+    # an API where these are optional would be too cumbersome,
+    # instead always pass in some dummy values
+    DUMMY_ALIASPAIR,
 
-      # not strictly necessary, but shouldn't hurt either
-      require_join_free_condition => !!(ref $rel_info->{cond} ne 'CODE'),
-    );
+    # this may look weird, but remember that we are making a resultset
+    # out of an existing object, with the new source being at the head
+    # of the FROM chain. Having a 'me' alias is nothing but expected there
+    foreign_alias => 'me',
+  };
 
-    # keep in mind that the following if() block is part of a do{} - no return()s!!!
-    if (
-      ! $cond_res->{join_free_condition}
-        and
-      ref $rel_info->{cond} eq 'CODE'
-    ) {
+  my $jfc = (
+    # In certain extraordinary circumstances the relationship resolution may
+    # throw (e.g. when walking through elaborate custom conds)
+    # In case the object is "real" (i.e. in_storage) we just go ahead and
+    # let the exception surface. Otherwise we carp and move on.
+    #
+    # The elaborate code-duplicating ternary is there because the xsified
+    # ->in_storage() is orders of magnitude faster than the Try::Tiny-like
+    # construct below ( perl's low level tooling is truly shit :/ )
+    ( $self->in_storage or DBIx::Class::_Util::in_internal_try )
+      ? $rsrc->resolve_relationship_condition($rrc_args)->{join_free_condition}
+      : dbic_internal_try {
+          $rsrc->resolve_relationship_condition($rrc_args)->{join_free_condition}
+        }
+        dbic_internal_catch {
+          $unique_carper->(
+            "Resolution of relationship '$rel' failed unexpectedly, "
+          . 'please relay the following error and seek assistance via '
+          . DBIx::Class::_ENV_::HELP_URL . ". Encountered error: $_"
+          );
+
+          # FIXME - this is questionable
+          # force skipping re-resolution, and instead just return an UC rset
+          $relcond_is_freeform = 0;
+
+          # RV
+          undef;
+        }
+  );
 
-      # A WHOREIFFIC hack to reinvoke the entire condition resolution
-      # with the correct alias. Another way of doing this involves a
-      # lot of state passing around, and the @_ positions are already
-      # mapped out, making this crap a less icky option.
-      #
-      # The point of this exercise is to retain the spirit of the original
-      # $obj->search_related($rel) where the resulting rset will have the
-      # root alias as 'me', instead of $rel (as opposed to invoking
-      # $rs->search_related)
-
-      # make the fake 'me' rel
-      local $rsrc->{_relationships}{me} = {
-        %{ $rsrc->{_relationships}{$rel} },
-        _original_name => $rel,
-      };
+  my $rel_rset;
 
-      my $obj_table_alias = lc($rsrc->source_name) . '__row';
-      $obj_table_alias =~ s/\W+/_/g;
+  if( defined $jfc ) {
 
-      $rsrc->resultset->search(
-        $self->ident_condition($obj_table_alias),
-        { alias => $obj_table_alias },
-      )->related_resultset('me')->search(undef, $rel_info->{attrs})
-    }
-    else {
-
-      # FIXME - this conditional doesn't seem correct - got to figure out
-      # at some point what it does. Also the entire UNRESOLVABLE_CONDITION
-      # business seems shady - we could simply not query *at all*
-      my $attrs;
-      if ( $cond_res->{join_free_condition} eq UNRESOLVABLE_CONDITION ) {
-        $attrs = { %{$rel_info->{attrs}} };
-        my $reverse = $rsrc->reverse_relationship_info($rel);
-        foreach my $rev_rel (keys %$reverse) {
-          if ($reverse->{$rev_rel}{attrs}{accessor} && $reverse->{$rev_rel}{attrs}{accessor} eq 'multi') {
-            weaken($attrs->{related_objects}{$rev_rel}[0] = $self);
-          } else {
-            weaken($attrs->{related_objects}{$rev_rel} = $self);
-          }
-        }
-      }
+    $rel_rset = $rsrc->related_source($rel)->resultset->search(
+      $jfc,
+      $rel_info->{attrs},
+    );
+  }
+  elsif( $relcond_is_freeform ) {
+
+    # A WHOREIFFIC hack to reinvoke the entire condition resolution
+    # with the correct alias. Another way of doing this involves a
+    # lot of state passing around, and the @_ positions are already
+    # mapped out, making this crap a less icky option.
+    #
+    # The point of this exercise is to retain the spirit of the original
+    # $obj->search_related($rel) where the resulting rset will have the
+    # root alias as 'me', instead of $rel (as opposed to invoking
+    # $rs->search_related)
+
+    # make the fake 'me' rel
+    local $rsrc->{_relationships}{me} = {
+      %{ $rsrc->{_relationships}{$rel} },
+      _original_name => $rel,
+    };
+
+    my $obj_table_alias = lc($rsrc->source_name) . '__row';
+    $obj_table_alias =~ s/\W+/_/g;
+
+    $rel_rset = $rsrc->resultset->search(
+      $self->ident_condition($obj_table_alias),
+      { alias => $obj_table_alias },
+    )->related_resultset('me')->search(undef, $rel_info->{attrs})
+  }
+  else {
+
+    my $attrs = { %{$rel_info->{attrs}} };
+    my $reverse = $rsrc->reverse_relationship_info($rel);
+
+    # FIXME - this loop doesn't seem correct - got to figure out
+    # at some point what exactly it does.
+    # See also the FIXME at the end of new_related()
+    ( ( $reverse->{$_}{attrs}{accessor}||'') eq 'multi' )
+      ? weaken( $attrs->{related_objects}{$_}[0] = $self )
+      : weaken( $attrs->{related_objects}{$_}    = $self )
+    for keys %$reverse;
+
+    $rel_rset = $rsrc->related_source($rel)->resultset->search(
+      UNRESOLVABLE_CONDITION, # guards potential use of the $rs in the future
+      $attrs,
+    );
+  }
 
-      $rsrc->related_source($rel)->resultset->search(
-        $cond_res->{join_free_condition},
-        $attrs || $rel_info->{attrs},
-      );
-    }
-  };
+  $self->{related_resultsets}{$rel} = $rel_rset;
 }
 
 =head2 search_related
@@ -667,13 +714,94 @@ your storage until you call L<DBIx::Class::Row/insert> on it.
 sub new_related {
   my ($self, $rel, $data) = @_;
 
-  $self->related_resultset($rel)->new_result( $self->result_source->_resolve_relationship_condition (
-    infer_values_based_on => $data,
+  $self->throw_exception(
+    "Result object instantiation requires a hashref as argument"
+  ) unless ref $data eq 'HASH';
+
+  my $rsrc = $self->result_source;
+  my $rel_rsrc = $rsrc->related_source($rel);
+
+###
+### This section deliberately does not rely on require_join_free_values,
+### as quite often the resulting related object is useless without the
+### contents of $data mixed in. Originally this code was part of
+### resolve_relationship_condition() but given it has a single, very
+### context-specific call-site it made no sense to expose it to end users.
+###
+
+  my $rel_resolution = $rsrc->resolve_relationship_condition (
     rel_name => $rel,
     self_result_object => $self,
-    foreign_alias => $rel,
-    self_alias => 'me',
-  )->{inferred_values} );
+
+    # In case we are *not* in_storage it is ok to treat failed resolution as an empty hash
+    # This happens e.g. as a result of various in-memory related graph of objects
+    require_join_free_condition => !! $self->in_storage,
+
+    # dummy aliases with deliberately known lengths, so that we can
+    # quickly strip them below if needed
+    foreign_alias => 'F',
+    self_alias    => 'S',
+  );
+
+  my $rel_values =
+    $rel_resolution->{join_free_values}
+      ||
+    { map { substr( $_, 2 ) => $rel_resolution->{join_free_condition}{$_} } keys %{ $rel_resolution->{join_free_condition} } }
+  ;
+
+  # mix everything together
+  my $amalgamated_values = {
+    %{
+      # in case we got back join_free_values - they already have passed the extractor
+      $rel_resolution->{join_free_values}
+        ? $rel_values
+        : extract_equality_conditions(
+          $rel_values,
+          'consider_nulls'
+        )
+    },
+    %$data,
+  };
+
+  # cleanup possible rogue { somecolumn => [ -and => 1,2 ] }
+  ($amalgamated_values->{$_}||'') eq UNRESOLVABLE_CONDITION
+    and
+  delete $amalgamated_values->{$_}
+    for keys %$amalgamated_values;
+
+  if( my @nonvalues = grep { ! exists $amalgamated_values->{$_} } keys %$rel_values ) {
+
+    $self->throw_exception(
+      "Unable to complete value inferrence - relationship '$rel' "
+    . "on source '@{[ $rsrc->source_name ]}' results "
+    . 'in expression(s) instead of definitive values: '
+    . do {
+        # FIXME - used for diag only, but still icky
+        my $sqlm =
+          dbic_internal_try { $rsrc->schema->storage->sql_maker }
+            ||
+          (
+            require DBIx::Class::SQLMaker
+              and
+            DBIx::Class::SQLMaker->new
+          )
+        ;
+        local $sqlm->{quote_char};
+        local $sqlm->{_dequalify_idents} = 1;
+        ($sqlm->_recurse_where({ map { $_ => $rel_values->{$_} } @nonvalues }))[0]
+      }
+    );
+  }
+
+  # And more complications - in case the relationship did not resolve
+  # we *have* to loop things through search_related ( essentially re-resolving
+  # everything we did so far, but with different type of handholding )
+  # FIXME - this is still a mess, just a *little* better than it was
+  # See also the FIXME at the end of related_resultset()
+  exists $rel_resolution->{join_free_values}
+    ? $rel_rsrc->result_class->new({ -result_source => $rel_rsrc, %$amalgamated_values })
+    : $self->related_resultset($rel)->new_result( $amalgamated_values )
+  ;
 }
 
 =head2 create_related
@@ -819,13 +947,43 @@ L<update|DBIx::Class::Row/update> to update them in the storage.
 sub set_from_related {
   my ($self, $rel, $f_obj) = @_;
 
-  $self->set_columns( $self->result_source->_resolve_relationship_condition (
-    infer_values_based_on => {},
+  $self->set_columns( $self->result_source->resolve_relationship_condition (
+    require_join_free_values => 1,
     rel_name => $rel,
-    foreign_values => $f_obj,
-    foreign_alias => $rel,
-    self_alias => 'me',
-  )->{inferred_values} );
+    foreign_values => (
+      # maintain crazy set_from_related interface
+      #
+      ( ! defined $f_obj )          ? +{}
+    : ( ! defined blessed $f_obj )  ? $f_obj
+                                    : do {
+
+        my $f_result_class = $self->result_source->related_source($rel)->result_class;
+
+        unless( $f_obj->isa($f_result_class) ) {
+
+          $self->throw_exception(
+            'Object supplied to set_from_related() must inherit from '
+          . "'$DBIx::Class::ResultSource::__expected_result_class_isa'"
+          ) unless $f_obj->isa(
+            $DBIx::Class::ResultSource::__expected_result_class_isa
+          );
+
+          carp_unique(
+            'Object supplied to set_from_related() usually should inherit from '
+          . "the related ResultClass ('$f_result_class'), perhaps you've made "
+          . 'a mistake?'
+          );
+        }
+
+        +{ $f_obj->get_columns };
+      }
+    ),
+
+    # an API where these are optional would be too cumbersome,
+    # instead always pass in some dummy values
+    DUMMY_ALIASPAIR,
+
+  )->{join_free_values} );
 
   return 1;
 }
index e715f10..2812b6b 100644 (file)
@@ -7,9 +7,10 @@ use warnings;
 use DBIx::Class::Carp;
 use DBIx::Class::_Util qw( quote_sub perlstring );
 
-# FIXME - this souldn't be needed
-my $cu;
-BEGIN { $cu = \&carp_unique }
+# FIXME - this should go away
+# instead Carp::Skip should export usable keywords or something like that
+my $unique_carper;
+BEGIN { $unique_carper = \&carp_unique }
 
 use namespace::clean;
 
@@ -82,7 +83,7 @@ EOC
     my @extra_meth_qsub_args = (
       {
         '$rel_attrs' => \{ alias => $f_rel, %{ $rel_attrs||{} } },
-        '$carp_unique' => \$cu,
+        '$carp_unique' => \$unique_carper,
       },
       { attributes => [
         'DBIC_method_is_indirect_sugar',
index bf7e88f..1d6d177 100644 (file)
@@ -9,11 +9,14 @@ 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_wantarray 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
@@ -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} },
+      };
     }
   }
 
@@ -2220,6 +2258,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 +2267,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 +2277,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 +2491,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,10 +2531,12 @@ 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} || {} } };
 
         }
@@ -3269,13 +3310,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 +3322,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};
index 0c0cb9d..ddef544 100644 (file)
@@ -17,11 +17,12 @@ use base 'DBIx::Class::ResultSource::RowParser';
 
 use DBIx::Class::Carp;
 use DBIx::Class::_Util qw(
-  UNRESOLVABLE_CONDITION
+  UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR
   dbic_internal_try fail_on_internal_call
-  refdesc emit_loud_diag
+  refdesc emit_loud_diag dump_value serialize bag_eq
 );
 use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use SQL::Abstract 'is_literal_value';
 use Devel::GlobalDestruction;
 use Scalar::Util qw( blessed weaken isweak refaddr );
@@ -1823,85 +1824,111 @@ L</relationship_info>.
 sub reverse_relationship_info {
   my ($self, $rel) = @_;
 
-  my $rel_info = $self->relationship_info($rel)
-    or $self->throw_exception("No such relationship '$rel'");
+  # This may be a partial schema or something else equally esoteric
+  # in which case this will throw
+  #
+  my $other_rsrc = $self->related_source($rel);
 
-  my $ret = {};
+  # Some custom rels may not resolve without a $schema
+  #
+  my $our_resolved_relcond = dbic_internal_try {
+    $self->resolve_relationship_condition(
+      rel_name => $rel,
 
-  return $ret unless ((ref $rel_info->{cond}) eq 'HASH');
+      # an API where these are optional would be too cumbersome,
+      # instead always pass in some dummy values
+      DUMMY_ALIASPAIR,
+    )
+  };
 
-  my $stripped_cond = $self->__strip_relcond ($rel_info->{cond});
+  # only straight-equality is compared
+  return {}
+    unless $our_resolved_relcond->{identity_map_matches_condition};
 
-  my $registered_source_name = $self->source_name;
+  my( $our_registered_source_name, $our_result_class) =
+    ( $self->source_name, $self->result_class );
 
-  # this may be a partial schema or something else equally esoteric
-  my $other_rsrc = $self->related_source($rel);
+  my $ret = {};
 
   # Get all the relationships for that source that related to this source
   # whose foreign column set are our self columns on $rel and whose self
   # columns are our foreign columns on $rel
   foreach my $other_rel ($other_rsrc->relationships) {
 
+    # this will happen when we have a self-referential class
+    next if (
+      $other_rel eq $rel
+        and
+      $self == $other_rsrc
+    );
+
     # only consider stuff that points back to us
     # "us" here is tricky - if we are in a schema registration, we want
     # to use the source_names, otherwise we will use the actual classes
 
-    # the schema may be partial
-    my $roundtrip_rsrc = dbic_internal_try { $other_rsrc->related_source($other_rel) }
-      or next;
+    my $roundtripped_rsrc;
+    next unless (
 
-    if ($registered_source_name) {
-      next if $registered_source_name ne ($roundtrip_rsrc->source_name || '')
-    }
-    else {
-      next if $self->result_class ne $roundtrip_rsrc->result_class;
-    }
+      # the schema may be partially loaded
+      $roundtripped_rsrc = dbic_internal_try { $other_rsrc->related_source($other_rel) }
 
-    my $other_rel_info = $other_rsrc->relationship_info($other_rel);
+        and
 
-    # this can happen when we have a self-referential class
-    next if $other_rel_info eq $rel_info;
+      (
 
-    next unless ref $other_rel_info->{cond} eq 'HASH';
-    my $other_stripped_cond = $self->__strip_relcond($other_rel_info->{cond});
+        (
+          $our_registered_source_name
+            and
+          (
+            $our_registered_source_name
+              eq
+            $roundtripped_rsrc->source_name||''
+          )
+        )
 
-    $ret->{$other_rel} = $other_rel_info if (
-      $self->_compare_relationship_keys (
-        [ keys %$stripped_cond ], [ values %$other_stripped_cond ]
+          or
+
+        (
+          $our_result_class
+            eq
+          $roundtripped_rsrc->result_class
+        )
       )
+
         and
-      $self->_compare_relationship_keys (
-        [ values %$stripped_cond ], [ keys %$other_stripped_cond ]
-      )
+
+      my $their_resolved_relcond = dbic_internal_try {
+        $other_rsrc->resolve_relationship_condition(
+          rel_name => $other_rel,
+
+          # an API where these are optional would be too cumbersome,
+          # instead always pass in some dummy values
+          DUMMY_ALIASPAIR,
+        )
+      }
     );
-  }
 
-  return $ret;
-}
 
-# all this does is removes the foreign/self prefix from a condition
-sub __strip_relcond {
-  +{
-    map
-      { map { /^ (?:foreign|self) \. (\w+) $/x } ($_, $_[1]{$_}) }
-      keys %{$_[1]}
-  }
-}
+    $ret->{$other_rel} = $other_rsrc->relationship_info($other_rel) if (
 
-sub compare_relationship_keys {
-  carp 'compare_relationship_keys is a private method, stop calling it';
-  my $self = shift;
-  $self->_compare_relationship_keys (@_);
-}
+      $their_resolved_relcond->{identity_map_matches_condition}
 
-# Returns true if both sets of keynames are the same, false otherwise.
-sub _compare_relationship_keys {
-#  my ($self, $keys1, $keys2) = @_;
-  return
-    join ("\x00", sort @{$_[1]})
-      eq
-    join ("\x00", sort @{$_[2]})
-  ;
+        and
+
+      keys %{ $our_resolved_relcond->{identity_map} }
+        ==
+      keys %{ $their_resolved_relcond->{identity_map} }
+
+        and
+
+      serialize( $our_resolved_relcond->{identity_map} )
+        eq
+      serialize( { reverse %{ $their_resolved_relcond->{identity_map} } } )
+
+    );
+  }
+
+  return $ret;
 }
 
 # optionally takes either an arrayref of column names, or a hashref of already
@@ -2069,7 +2096,7 @@ sub _resolve_join {
                -alias => $as,
                -relation_chain_depth => ( $seen->{-relation_chain_depth} || 0 ) + 1,
              },
-             $self->_resolve_relationship_condition(
+             $self->resolve_relationship_condition(
                rel_name => $join,
                self_alias => $alias,
                foreign_alias => $as,
@@ -2123,18 +2150,48 @@ sub _pk_depends_on {
   return 1;
 }
 
-sub resolve_condition {
-  carp 'resolve_condition is a private method, stop calling it';
+sub __strip_relcond :DBIC_method_is_indirect_sugar {
+  DBIx::Class::Exception->throw(
+    '__strip_relcond() has been removed with no replacement, '
+  . 'ask for advice on IRC if this affected you'
+  );
+}
+
+sub compare_relationship_keys :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique( 'compare_relationship_keys() is deprecated, ask on IRC for a better alternative' );
+  bag_eq( $_[1], $_[2] );
+}
+
+sub _compare_relationship_keys :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique( '_compare_relationship_keys() is deprecated, ask on IRC for a better alternative' );
+  bag_eq( $_[1], $_[2] );
+}
+
+sub _resolve_relationship_condition :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
+  # carp() - has been on CPAN for less than 2 years
+  carp '_resolve_relationship_condition() is deprecated - see resolve_relationship_condition() instead';
+
+  shift->resolve_relationship_condition(@_);
+}
+
+sub resolve_condition :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
+  # carp() - has been discouraged forever
+  carp 'resolve_condition() is deprecated - see resolve_relationship_condition() instead';
+
   shift->_resolve_condition (@_);
 }
 
-sub _resolve_condition {
-#  carp_unique sprintf
-#    '_resolve_condition is a private method, and moreover is about to go '
-#  . 'away. Please contact the development team at %s if you believe you '
-#  . 'have a genuine use for this method, in order to discuss alternatives.',
-#    DBIx::Class::_ENV_::HELP_URL,
-#  ;
+sub _resolve_condition :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+
+  # carp_unique() - the interface replacing it only became reality in Sep 2016
+  carp_unique '_resolve_condition() is deprecated - see resolve_relationship_condition() instead';
 
 #######################
 ### API Design? What's that...? (a backwards compatible shim, kill me now)
@@ -2159,6 +2216,10 @@ sub _resolve_condition {
         $is_objlike[$_] = 0;
         $res_args[$_] = '__gremlins__';
       }
+      # more compat
+      elsif( $_ == 0 and $res_args[0]->isa( $__expected_result_class_isa ) ) {
+        $res_args[0] = { $res_args[0]->get_columns };
+      }
     }
     else {
       $res_args[$_] ||= {};
@@ -2182,21 +2243,21 @@ sub _resolve_condition {
   };
 
   # Allowing passing relconds different than the relationshup itself is cute,
-  # but likely dangerous. Remove that from the (still unofficial) API of
-  # _resolve_relationship_condition, and instead make it "hard on purpose"
+  # but likely dangerous. Remove that from the API of resolve_relationship_condition,
+  # and instead make it "hard on purpose"
   local $self->relationship_info( $args->{rel_name} )->{cond} = $cond if defined $cond;
 
 #######################
 
   # now it's fucking easy isn't it?!
-  my $rc = $self->_resolve_relationship_condition( $args );
+  my $rc = $self->resolve_relationship_condition( $args );
 
   my @res = (
     ( $rc->{join_free_condition} || $rc->{condition} ),
     ! $rc->{join_free_condition},
   );
 
-  # _resolve_relationship_condition always returns qualified cols even in the
+  # resolve_relationship_condition always returns qualified cols even in the
   # case of join_free_condition, but nothing downstream expects this
   if ($rc->{join_free_condition} and ref $res[0] eq 'HASH') {
     $res[0] = { map
@@ -2218,34 +2279,73 @@ our $UNRESOLVABLE_CONDITION = UNRESOLVABLE_CONDITION;
 # we are moving to a constant
 Internals::SvREADONLY($UNRESOLVABLE_CONDITION => 1);
 
-# Resolves the passed condition to a concrete query fragment and extra
-# metadata
-#
-## self-explanatory API, modeled on the custom cond coderef:
-# rel_name              => (scalar)
-# foreign_alias         => (scalar)
-# foreign_values        => (either not supplied, or a hashref, or a foreign ResultObject (to be ->get_columns()ed), or plain undef )
-# self_alias            => (scalar)
-# self_result_object    => (either not supplied or a result object)
-# require_join_free_condition => (boolean, throws on failure to construct a JF-cond)
-# infer_values_based_on => (either not supplied or a hashref, implies require_join_free_condition)
-#
-## returns a hash
-# condition           => (a valid *likely fully qualified* sqla cond structure)
-# identity_map        => (a hashref of foreign-to-self *unqualified* column equality names)
-# join_free_condition => (a valid *fully qualified* sqla cond structure, maybe unset)
-# inferred_values     => (in case of an available join_free condition, this is a hashref of
-#                         *unqualified* column/value *EQUALITY* pairs, representing an amalgamation
-#                         of the JF-cond parse and infer_values_based_on
-#                         always either complete or unset)
-#
-sub _resolve_relationship_condition {
+=head2 resolve_relationship_condition
+
+NOTE: You generally B<SHOULD NOT> need to use this functionality... until you
+do. The API description is terse on purpose. If the text below doesn't make
+sense right away (based on the context which prompted you to look here) it is
+almost certain you are reaching for the wrong tool. Please consider asking for
+advice in any of the support channels before proceeding.
+
+=over 4
+
+=item Arguments: C<\%args> as shown below (C<B<*>> denotes mandatory args):
+
+  * rel_name                    => $string
+
+  * foreign_alias               => $string
+
+  * self_alias                  => $string
+
+    foreign_values              => \%column_value_pairs
+
+    self_result_object          => $ResultObject
+
+    require_join_free_condition => $bool ( results in exception on failure to construct a JF-cond )
+
+    require_join_free_values    => $bool ( results in exception on failure to return an equality-only JF-cond )
+
+=item Return Value: C<\%resolution_result> as shown below (C<B<*>> denotes always-resent parts of the result):
+
+  * condition                      => $sqla_condition ( always present, valid, *likely* fully qualified, SQL::Abstract-compatible structure )
+
+    identity_map                   => \%foreign_to_self_equailty_map ( list of declared-equal foreign/self *unqualified* column names )
+
+    identity_map_matches_condition => $bool ( indicates whether the entire condition is expressed within the identity_map )
+
+    join_free_condition            => \%sqla_condition_fully_resolvable_via_foreign_table
+                                      ( always a hash, all keys guaranteed to be valid *fully qualified* columns )
+
+    join_free_values               => \%unqalified_version_of_join_free_condition
+                                      ( IFF the returned join_free_condition contains only exact values (no expressions), this would be
+                                        a hashref identical to join_free_condition, except with all column names *unqualified* )
+
+=back
+
+This is the low-level method used to convert a declared relationship into
+various parameters consumed by higher level functions. It is provided as a
+stable official API, as the logic it encapsulates grew incredibly complex with
+time. While calling this method directly B<is generally discouraged>, you
+absolutely B<should be using it> in codepaths containing the moral equivalent
+of:
+
+  ...
+  if( ref $some_rsrc->relationship_info($somerel)->{cond} eq 'HASH' ) {
+    ...
+  }
+  ...
+
+=cut
+
+# TODO - expand the documentation above, too terse
+
+sub resolve_relationship_condition {
   my $self = shift;
 
   my $args = { ref $_[0] eq 'HASH' ? %{ $_[0] } : @_ };
 
   for ( qw( rel_name self_alias foreign_alias ) ) {
-    $self->throw_exception("Mandatory argument '$_' to _resolve_relationship_condition() is not a plain string")
+    $self->throw_exception("Mandatory argument '$_' to resolve_relationship_condition() is not a plain string")
       if !defined $args->{$_} or length ref $args->{$_};
   }
 
@@ -2253,7 +2353,7 @@ sub _resolve_relationship_condition {
     if $args->{self_alias} eq $args->{foreign_alias};
 
 # TEMP
-  my $exception_rel_id = "relationship '$args->{rel_name}' on source '@{[ $self->source_name ]}'";
+  my $exception_rel_id = "relationship '$args->{rel_name}' on source '@{[ $self->source_name || $self->result_class ]}'";
 
   my $rel_info = $self->relationship_info($args->{rel_name})
 # TEMP
@@ -2267,10 +2367,7 @@ sub _resolve_relationship_condition {
   $self->throw_exception("No practical way to resolve $exception_rel_id between two data structures")
     if exists $args->{self_result_object} and exists $args->{foreign_values};
 
-  $self->throw_exception( "Argument to infer_values_based_on must be a hash" )
-    if exists $args->{infer_values_based_on} and ref $args->{infer_values_based_on} ne 'HASH';
-
-  $args->{require_join_free_condition} ||= !!$args->{infer_values_based_on};
+  $args->{require_join_free_condition} ||= !!$args->{require_join_free_values};
 
   $self->throw_exception( "Argument 'self_result_object' must be an object inheriting from '$__expected_result_class_isa'" )
     if (
@@ -2285,69 +2382,79 @@ sub _resolve_relationship_condition {
   ;
 
   my $rel_rsrc = $self->related_source($args->{rel_name});
-  my $storage = $self->schema->storage;
-
-  if (exists $args->{foreign_values}) {
-
-    if (! defined $args->{foreign_values} ) {
-      # fallback: undef => {}
-      $args->{foreign_values} = {};
-    }
-    elsif (defined blessed $args->{foreign_values}) {
-
-      $self->throw_exception( "Objects supplied as 'foreign_values' ($args->{foreign_values}) must inherit from '$__expected_result_class_isa'" )
-        unless $args->{foreign_values}->isa( $__expected_result_class_isa );
-
-      carp_unique(
-        "Objects supplied as 'foreign_values' ($args->{foreign_values}) "
-      . "usually should inherit from the related ResultClass ('@{[ $rel_rsrc->result_class ]}'), "
-      . "perhaps you've made a mistake invoking the condition resolver?"
-      ) unless $args->{foreign_values}->isa($rel_rsrc->result_class);
-
-      $args->{foreign_values} = { $args->{foreign_values}->get_columns };
-    }
-    elsif ( ref $args->{foreign_values} eq 'HASH' ) {
 
-      # re-build {foreign_values} excluding identically named rels
-      if( keys %{$args->{foreign_values}} ) {
+  if (
+    exists $args->{foreign_values}
+      and
+    (
+      ref $args->{foreign_values} eq 'HASH'
+        or
+      $self->throw_exception(
+        "Argument 'foreign_values' must be a hash reference"
+      )
+    )
+      and
+    keys %{$args->{foreign_values}}
+  ) {
 
-        my ($col_idx, $rel_idx) = map
-          { { map { $_ => 1 } $rel_rsrc->$_ } }
-          qw( columns relationships )
-        ;
+    my ($col_idx, $rel_idx) = map
+      { { map { $_ => 1 } $rel_rsrc->$_ } }
+      qw( columns relationships )
+    ;
 
-        my $equivalencies = extract_equality_conditions(
-          $args->{foreign_values},
-          'consider nulls',
-        );
+    my $equivalencies;
 
-        $args->{foreign_values} = { map {
-          # skip if relationship *and* a non-literal ref
-          # this means a multicreate stub was passed in
+    # re-build {foreign_values} excluding refs as follows
+    # ( hot codepath: intentionally convoluted )
+    #
+    $args->{foreign_values} = { map {
+      (
+        $_ !~ /^-/
+          or
+        $self->throw_exception(
+          "The key '$_' supplied as part of 'foreign_values' during "
+         . 'relationship resolution must be a column name, not a function'
+        )
+      )
+        and
+      (
+        # skip if relationship ( means a multicreate stub was passed in )
+        # skip if literal ( can't infer anything about it )
+        # or plain throw if nonequiv yet not literal
+        (
+          length ref $args->{foreign_values}{$_}
+            and
           (
             $rel_idx->{$_}
-              and
-            length ref $args->{foreign_values}{$_}
-              and
-            ! is_literal_value($args->{foreign_values}{$_})
+              or
+            is_literal_value($args->{foreign_values}{$_})
+              or
+            (
+              (
+                ! exists(
+                  ( $equivalencies ||= extract_equality_conditions( $args->{foreign_values}, 'consider nulls' ) )
+                    ->{$_}
+                )
+                  or
+                ($equivalencies->{$_}||'') eq UNRESOLVABLE_CONDITION
+              )
+                and
+              $self->throw_exception(
+                "Resolution of relationship '$args->{rel_name}' failed: "
+              . "supplied value for foreign column '$_' is not a direct "
+              . 'equivalence expression'
+              )
+            )
           )
-            ? ()
-            : ( $_ => (
-                ! $col_idx->{$_}
-                  ? $self->throw_exception( "Key '$_' supplied as 'foreign_values' is not a column on related source '@{[ $rel_rsrc->source_name ]}'" )
-              : ( !exists $equivalencies->{$_} or ($equivalencies->{$_}||'') eq UNRESOLVABLE_CONDITION )
-                  ? $self->throw_exception( "Value supplied for '...{foreign_values}{$_}' is not a direct equivalence expression" )
-              : $args->{foreign_values}{$_}
-            ))
-        } keys %{$args->{foreign_values}} };
-      }
-    }
-    else {
-      $self->throw_exception(
-        "Argument 'foreign_values' must be either an object inheriting from '@{[ $rel_rsrc->result_class ]}', "
-      . "or a hash reference, or undef"
-      );
-    }
+        )                             ? ()
+      : $col_idx->{$_}                ? ( $_ => $args->{foreign_values}{$_} )
+                                      : $self->throw_exception(
+            "The key '$_' supplied as part of 'foreign_values' during "
+           . 'relationship resolution is not a column on related source '
+           . "'@{[ $rel_rsrc->source_name ]}'"
+          )
+      )
+    } keys %{$args->{foreign_values}} };
   }
 
   my $ret;
@@ -2377,11 +2484,11 @@ sub _resolve_relationship_condition {
     $self->throw_exception("A custom condition coderef can return at most 2 conditions, but $exception_rel_id returned extra values: @extra")
       if @extra;
 
-    if (my $jfc = $ret->{join_free_condition}) {
+    if( $ret->{join_free_condition} ) {
 
       $self->throw_exception (
         "The join-free condition returned for $exception_rel_id must be a hash reference"
-      ) unless ref $jfc eq 'HASH';
+      ) unless ref $ret->{join_free_condition} eq 'HASH';
 
       my ($joinfree_alias, $joinfree_source);
       if (defined $args->{self_result_object}) {
@@ -2407,7 +2514,7 @@ sub _resolve_relationship_condition {
         "The join-free condition returned for $exception_rel_id may only "
       . 'contain keys that are fully qualified column names of the corresponding source '
       . "'$joinfree_alias' (instead it returned '$_')"
-      ) for keys %$jfc;
+      ) for keys %{$ret->{join_free_condition}};
 
       (
         defined blessed($_)
@@ -2419,7 +2526,7 @@ sub _resolve_relationship_condition {
         . 'contain result objects as values - perhaps instead of invoking '
         . '->$something you meant to return ->get_column($something)'
         )
-      ) for values %$jfc;
+      ) for values %{$ret->{join_free_condition}};
 
     }
   }
@@ -2446,61 +2553,76 @@ sub _resolve_relationship_condition {
     # construct the crosstable condition and the identity map
     for  (0..$#f_cols) {
       $ret->{condition}{"$args->{foreign_alias}.$f_cols[$_]"} = { -ident => "$args->{self_alias}.$l_cols[$_]" };
-      $ret->{identity_map}{$l_cols[$_]} = $f_cols[$_];
+
+      # explicit value stringification is deliberate - leave no room for
+      # interpretation when comparing sets of keys
+      $ret->{identity_map}{$l_cols[$_]} = "$f_cols[$_]";
     };
 
     if ($args->{foreign_values}) {
-      $ret->{join_free_condition}{"$args->{self_alias}.$l_cols[$_]"} = $args->{foreign_values}{$f_cols[$_]}
+      $ret->{join_free_condition}{"$args->{self_alias}.$l_cols[$_]"}
+        = $ret->{join_free_values}{$l_cols[$_]}
+          = $args->{foreign_values}{$f_cols[$_]}
         for 0..$#f_cols;
     }
     elsif (defined $args->{self_result_object}) {
 
-      for my $i (0..$#l_cols) {
-        if ( $args->{self_result_object}->has_column_loaded($l_cols[$i]) ) {
-          $ret->{join_free_condition}{"$args->{foreign_alias}.$f_cols[$i]"} = $args->{self_result_object}->get_column($l_cols[$i]);
-        }
-        else {
-          $self->throw_exception(sprintf
-            "Unable to resolve relationship '%s' from object '%s': column '%s' not "
-          . 'loaded from storage (or not passed to new() prior to insert()). You '
-          . 'probably need to call ->discard_changes to get the server-side defaults '
-          . 'from the database.',
-            $args->{rel_name},
-            $args->{self_result_object},
-            $l_cols[$i],
-          ) if $args->{self_result_object}->in_storage;
-
-          # FIXME - temporarly force-override
-          delete $args->{require_join_free_condition};
-          $ret->{join_free_condition} = UNRESOLVABLE_CONDITION;
-          last;
-        }
-      }
+      # FIXME - compat block due to inconsistency of get_columns() vs has_column_loaded()
+      # The former returns cached-in related single rels, while the latter is doing what
+      # it says on the tin. Thus the more logical "get all columns and barf if something
+      # is missing" is a non-starter, and we move through each column one by one :/
+
+      $args->{self_result_object}->has_column_loaded( $l_cols[$_] )
+
+            ? $ret->{join_free_condition}{"$args->{foreign_alias}.$f_cols[$_]"}
+                = $ret->{join_free_values}{$f_cols[$_]}
+                  = $args->{self_result_object}->get_column( $l_cols[$_] )
+
+    : $args->{self_result_object}->in_storage
+
+            ? $self->throw_exception(sprintf
+                "Unable to resolve relationship '%s' from object '%s': column '%s' not "
+              . 'loaded from storage (or not passed to new() prior to insert()). You '
+            . 'probably need to call ->discard_changes to get the server-side defaults '
+            . 'from the database',
+              $args->{rel_name},
+              $args->{self_result_object},
+              $l_cols[$_],
+            )
+
+      # non-resolvable yet not in storage - give it a pass
+      # FIXME - while this is what the code has done for ages, it doesn't seem right :(
+            : (
+              delete $ret->{join_free_condition},
+              delete $ret->{join_free_values},
+              last
+            )
+
+        for 0 .. $#l_cols;
     }
   }
   elsif (ref $rel_info->{cond} eq 'ARRAY') {
     if (@{ $rel_info->{cond} } == 0) {
       $ret = {
         condition => UNRESOLVABLE_CONDITION,
-        join_free_condition => UNRESOLVABLE_CONDITION,
       };
     }
     else {
       my @subconds = map {
         local $rel_info->{cond} = $_;
-        $self->_resolve_relationship_condition( $args );
+        $self->resolve_relationship_condition( $args );
       } @{ $rel_info->{cond} };
 
       if( @{ $rel_info->{cond} } == 1 ) {
         $ret = $subconds[0];
       }
       else {
-        # we are discarding inferred values here... likely incorrect...
-        # then again - the entire thing is an OR, so we *can't* use them anyway
         for my $subcond ( @subconds ) {
           $self->throw_exception('Either all or none of the OR-condition members must resolve to a join-free condition')
             if ( $ret and ( $ret->{join_free_condition} xor $subcond->{join_free_condition} ) );
 
+          # we are discarding join_free_values from individual 'OR' branches here
+          # see @nonvalues checks below
           $subcond->{$_} and push @{$ret->{$_}}, $subcond->{$_} for (qw(condition join_free_condition));
         }
       }
@@ -2510,10 +2632,23 @@ sub _resolve_relationship_condition {
     $self->throw_exception ("Can't handle condition $rel_info->{cond} for $exception_rel_id yet :(");
   }
 
+
+  # Explicit normalization pass
+  # ( nobody really knows what a CODE can return )
+  # Explicitly leave U_C alone - it would be normalized
+  # to an { -and => [ U_C ] }
+  defined $ret->{$_}
+    and
+  $ret->{$_} ne UNRESOLVABLE_CONDITION
+    and
+  $ret->{$_} = normalize_sqla_condition($ret->{$_})
+    for qw(condition join_free_condition);
+
+
   if (
     $args->{require_join_free_condition}
       and
-    ( ! $ret->{join_free_condition} or $ret->{join_free_condition} eq UNRESOLVABLE_CONDITION )
+    ! defined $ret->{join_free_condition}
   ) {
     $self->throw_exception(
       ucfirst sprintf "$exception_rel_id does not resolve to a %sjoin-free condition fragment",
@@ -2523,51 +2658,72 @@ sub _resolve_relationship_condition {
     );
   }
 
-  # we got something back - sanity check and infer values if we can
+  # we got something back (not from a static cond) - sanity check and infer values if we can
+  # ( in case of a static cond join_free_values is already pre-populated for us )
   my @nonvalues;
-  if (
+  if(
     $ret->{join_free_condition}
       and
-    $ret->{join_free_condition} ne UNRESOLVABLE_CONDITION
-      and
-    my $jfc = normalize_sqla_condition( $ret->{join_free_condition} )
+    ! $ret->{join_free_values}
   ) {
 
-    my $jfc_eqs = extract_equality_conditions( $jfc, 'consider_nulls' );
-
-    if (keys %$jfc_eqs) {
+    my $jfc_eqs = extract_equality_conditions(
+      $ret->{join_free_condition},
+      'consider_nulls'
+    );
 
-      for (keys %$jfc) {
-        # $jfc is fully qualified by definition
-        my ($col) = $_ =~ /\.(.+)/;
+    for( keys %{ $ret->{join_free_condition} } ) {
+      if( $_ =~ /^-/ ) {
+        push @nonvalues, { $_ => $ret->{join_free_condition}{$_} };
+      }
+      else {
+        # a join_free_condition is fully qualified by definition
+        my ($col) = $_ =~ /\.(.+)/ or carp_unique(
+          'Internal error - extract_equality_conditions() returned a '
+        . "non-fully-qualified key '$_'. *Please* file a bugreport "
+        . "including your definition of $exception_rel_id"
+        );
 
         if (exists $jfc_eqs->{$_} and ($jfc_eqs->{$_}||'') ne UNRESOLVABLE_CONDITION) {
-          $ret->{inferred_values}{$col} = $jfc_eqs->{$_};
+          $ret->{join_free_values}{$col} = $jfc_eqs->{$_};
         }
-        elsif ( !$args->{infer_values_based_on} or ! exists $args->{infer_values_based_on}{$col} ) {
-          push @nonvalues, $col;
+        else {
+          push @nonvalues, { $_ => $ret->{join_free_condition}{$_} };
         }
       }
-
-      # all or nothing
-      delete $ret->{inferred_values} if @nonvalues;
     }
-  }
 
-  # did the user explicitly ask
-  if ($args->{infer_values_based_on}) {
+    # all or nothing
+    delete $ret->{join_free_values} if @nonvalues;
+  }
 
-    $self->throw_exception(sprintf (
-      "Unable to complete value inferrence - custom $exception_rel_id returns conditions instead of values for column(s): %s",
-      map { "'$_'" } @nonvalues
-    )) if @nonvalues;
 
+  # throw only if the user explicitly asked
+  $args->{require_join_free_values}
+    and
+  @nonvalues
+    and
+  $self->throw_exception(
+    "Unable to complete value inferrence - $exception_rel_id results in expression(s) instead of definitive values: "
+  . do {
+      # FIXME - used for diag only, but still icky
+      my $sqlm =
+        dbic_internal_try { $self->schema->storage->sql_maker }
+          ||
+        (
+          require DBIx::Class::SQLMaker
+            and
+          DBIx::Class::SQLMaker->new
+        )
+      ;
+      local $sqlm->{quote_char};
+      local $sqlm->{_dequalify_idents} = 1;
+      ($sqlm->_recurse_where({ -and => \@nonvalues }))[0]
+    }
+  );
 
-    $ret->{inferred_values} ||= {};
 
-    $ret->{inferred_values}{$_} = $args->{infer_values_based_on}{$_}
-      for keys %{$args->{infer_values_based_on}};
-  }
+  my $identity_map_incomplete;
 
   # add the identities based on the main condition
   # (may already be there, since easy to calculate on the fly in the HASH case)
@@ -2575,32 +2731,60 @@ sub _resolve_relationship_condition {
 
     my $col_eqs = extract_equality_conditions($ret->{condition});
 
+    $identity_map_incomplete++ if (
+      $ret->{condition} eq UNRESOLVABLE_CONDITION
+        or
+      (
+        keys %{$ret->{condition}}
+          !=
+        keys %$col_eqs
+      )
+    );
+
     my $colinfos;
     for my $lhs (keys %$col_eqs) {
 
+      # start with the assumption it won't work
+      $identity_map_incomplete++;
+
       next if $col_eqs->{$lhs} eq UNRESOLVABLE_CONDITION;
 
       # there is no way to know who is right and who is left in a cref
       # therefore a full blown resolution call, and figure out the
       # direction a bit further below
-      $colinfos ||= $storage->_resolve_column_info([
+      $colinfos ||= fromspec_columns_info([
         { -alias => $args->{self_alias}, -rsrc => $self },
         { -alias => $args->{foreign_alias}, -rsrc => $rel_rsrc },
       ]);
 
       next unless $colinfos->{$lhs};  # someone is engaging in witchcraft
 
-      if ( my $rhs_ref = is_literal_value( $col_eqs->{$lhs} ) ) {
-
+      if( my $rhs_ref =
+        (
+          ref $col_eqs->{$lhs} eq 'HASH'
+            and
+          keys %{$col_eqs->{$lhs}} == 1
+            and
+          exists $col_eqs->{$lhs}{-ident}
+        )
+          ? [ $col_eqs->{$lhs}{-ident} ]  # repack to match the RV of is_literal_value
+          : is_literal_value( $col_eqs->{$lhs} )
+      ) {
         if (
           $colinfos->{$rhs_ref->[0]}
             and
           $colinfos->{$lhs}{-source_alias} ne $colinfos->{$rhs_ref->[0]}{-source_alias}
         ) {
           ( $colinfos->{$lhs}{-source_alias} eq $args->{self_alias} )
-            ? ( $ret->{identity_map}{$colinfos->{$lhs}{-colname}} = $colinfos->{$rhs_ref->[0]}{-colname} )
-            : ( $ret->{identity_map}{$colinfos->{$rhs_ref->[0]}{-colname}} = $colinfos->{$lhs}{-colname} )
+
+            # explicit value stringification is deliberate - leave no room for
+            # interpretation when comparing sets of keys
+            ? ( $ret->{identity_map}{$colinfos->{$lhs}{-colname}} = "$colinfos->{$rhs_ref->[0]}{-colname}" )
+            : ( $ret->{identity_map}{$colinfos->{$rhs_ref->[0]}{-colname}} = "$colinfos->{$lhs}{-colname}" )
           ;
+
+          # well, what do you know!
+          $identity_map_incomplete--;
         }
       }
       elsif (
@@ -2620,9 +2804,101 @@ sub _resolve_relationship_condition {
     }
   }
 
+  $ret->{identity_map_matches_condition} = ($identity_map_incomplete ? 0 : 1)
+    if $ret->{identity_map};
+
+
+  # cleanup before final return, easier to eyeball
+  ! defined $ret->{$_} and delete $ret->{$_}
+    for keys %$ret;
+
+
   # FIXME - temporary, to fool the idiotic check in SQLMaker::_join_condition
-  $ret->{condition} = { -and => [ $ret->{condition} ] }
-    unless $ret->{condition} eq UNRESOLVABLE_CONDITION;
+  $ret->{condition} = { -and => [ $ret->{condition} ] } unless (
+    $ret->{condition} eq UNRESOLVABLE_CONDITION
+      or
+    (
+      ref $ret->{condition} eq 'HASH'
+        and
+      grep { $_ =~ /^-/ } keys %{$ret->{condition}}
+    )
+  );
+
+
+  if( DBIx::Class::_ENV_::ASSERT_NO_INCONSISTENT_RELATIONSHIP_RESOLUTION ) {
+
+    my $sqlm =
+      dbic_internal_try { $self->schema->storage->sql_maker }
+        ||
+      (
+        require DBIx::Class::SQLMaker
+          and
+        DBIx::Class::SQLMaker->new
+      )
+    ;
+
+    local $sqlm->{_dequalify_idents} = 1;
+
+    my ( $cond_as_sql, $jf_cond_as_sql, $jf_vals_as_sql, $identmap_as_sql ) = map
+      { join ' : ', map {
+        ref $_ eq 'ARRAY' ? $_->[1]
+      : defined $_        ? $_
+                          : '{UNDEF}'
+      } $sqlm->_recurse_where($_) }
+      (
+        ( map { $ret->{$_} } qw( condition join_free_condition join_free_values ) ),
+
+        { map {
+          # inverse because of how the idmap is declared
+          $ret->{identity_map}{$_} => { -ident => $_ }
+        } keys %{$ret->{identity_map}} },
+      )
+    ;
+
+
+    emit_loud_diag(
+      confess => 1,
+      msg => sprintf (
+        "Resolution of %s produced inconsistent metadata:\n\n"
+      . "returned value of 'identity_map_matches_condition':    %s\n"
+      . "returned 'condition' rendered as de-qualified SQL:     %s\n"
+      . "returned 'identity_map' rendered as de-qualified SQL:  %s\n\n"
+      . "The condition declared on the misclassified relationship is: %s ",
+        $exception_rel_id,
+        ( $ret->{identity_map_matches_condition} || 0 ),
+        $cond_as_sql,
+        $identmap_as_sql,
+        dump_value( $rel_info->{cond} ),
+      ),
+    ) if (
+      $ret->{identity_map_matches_condition}
+        xor
+      ( $cond_as_sql eq $identmap_as_sql )
+    );
+
+
+    emit_loud_diag(
+      confess => 1,
+      msg => sprintf (
+        "Resolution of %s produced inconsistent metadata:\n\n"
+      . "returned 'join_free_condition' rendered as de-qualified SQL: %s\n"
+      . "returned 'join_free_values' rendered as de-qualified SQL:    %s\n\n"
+      . "The condition declared on the misclassified relationship is: %s ",
+        $exception_rel_id,
+        $jf_cond_as_sql,
+        $jf_vals_as_sql,
+        dump_value( $rel_info->{cond} ),
+      ),
+    ) if (
+      exists $ret->{join_free_condition}
+        and
+      (
+        exists $ret->{join_free_values}
+          xor
+        ( $jf_cond_as_sql eq $jf_vals_as_sql )
+      )
+    );
+  }
 
   $ret;
 }
diff --git a/lib/DBIx/Class/ResultSource/FromSpec/Util.pm b/lib/DBIx/Class/ResultSource/FromSpec/Util.pm
new file mode 100644 (file)
index 0000000..47106d7
--- /dev/null
@@ -0,0 +1,140 @@
+package   #hide from PAUSE
+  DBIx::Class::ResultSource::FromSpec::Util;
+
+use strict;
+use warnings;
+
+use base 'Exporter';
+our @EXPORT_OK = qw(
+  fromspec_columns_info
+  find_join_path_to_alias
+);
+
+use Scalar::Util 'blessed';
+
+# Takes $fromspec, \@column_names
+#
+# returns { $column_name => \%column_info, ... } for fully qualified and
+# where possible also unqualified variants
+# also note: this adds -result_source => $rsrc to the column info
+#
+# If no columns_names are supplied returns info about *all* columns
+# for all sources
+sub fromspec_columns_info {
+  my ($fromspec, $colnames) = @_;
+
+  return {} if $colnames and ! @$colnames;
+
+  my $sources = (
+    # this is compat mode for insert/update/delete which do not deal with aliases
+    (
+      blessed($fromspec)
+        and
+      $fromspec->isa('DBIx::Class::ResultSource')
+    )                                                 ? +{ me => $fromspec }
+
+    # not a known fromspec - no columns to resolve: return directly
+  : ref($fromspec) ne 'ARRAY'                         ? return +{}
+
+                                                      : +{
+    # otherwise decompose into alias/rsrc pairs
+      map
+        {
+          ( $_->{-rsrc} and $_->{-alias} )
+            ? ( @{$_}{qw( -alias -rsrc )} )
+            : ()
+        }
+        map
+          {
+            ( ref $_ eq 'ARRAY' and ref $_->[0] eq 'HASH' ) ? $_->[0]
+          : ( ref $_ eq 'HASH' )                            ? $_
+                                                            : ()
+          }
+          @$fromspec
+    }
+  );
+
+  $_ = { rsrc => $_, colinfos => $_->columns_info }
+    for values %$sources;
+
+  my (%seen_cols, @auto_colnames);
+
+  # compile a global list of column names, to be able to properly
+  # disambiguate unqualified column names (if at all possible)
+  for my $alias (keys %$sources) {
+    (
+      ++$seen_cols{$_}{$alias}
+        and
+      ! $colnames
+        and
+      push @auto_colnames, "$alias.$_"
+    ) for keys %{ $sources->{$alias}{colinfos} };
+  }
+
+  $colnames ||= [
+    @auto_colnames,
+    ( grep { keys %{$seen_cols{$_}} == 1 } keys %seen_cols ),
+  ];
+
+  my %return;
+  for (@$colnames) {
+    my ($colname, $source_alias) = reverse split /\./, $_;
+
+    my $assumed_alias =
+      $source_alias
+        ||
+      # if the column was seen exactly once - we know which rsrc it came from
+      (
+        $seen_cols{$colname}
+          and
+        keys %{$seen_cols{$colname}} == 1
+          and
+        ( %{$seen_cols{$colname}} )[0]
+      )
+        ||
+      next
+    ;
+
+    DBIx::Class::Exception->throw(
+      "No such column '$colname' on source " . $sources->{$assumed_alias}{rsrc}->source_name
+    ) unless $seen_cols{$colname}{$assumed_alias};
+
+    $return{$_} = {
+      %{ $sources->{$assumed_alias}{colinfos}{$colname} },
+      -result_source => $sources->{$assumed_alias}{rsrc},
+      -source_alias => $assumed_alias,
+      -fq_colname => "$assumed_alias.$colname",
+      -colname => $colname,
+    };
+
+    $return{"$assumed_alias.$colname"} = $return{$_}
+      unless $source_alias;
+  }
+
+  \%return;
+}
+
+sub find_join_path_to_alias {
+  my ($fromspec, $target_alias) = @_;
+
+  # subqueries and other oddness are naturally not supported
+  return undef if (
+    ref $fromspec ne 'ARRAY'
+      ||
+    ref $fromspec->[0] ne 'HASH'
+      ||
+    ! defined $fromspec->[0]{-alias}
+  );
+
+  # no path - the head *is* the alias
+  return [] if $fromspec->[0]{-alias} eq $target_alias;
+
+  for my $i (1 .. $#$fromspec) {
+    return $fromspec->[$i][0]{-join_path} if ( ($fromspec->[$i][0]{-alias}||'') eq $target_alias );
+  }
+
+  # something else went quite wrong
+  return undef;
+}
+
+1;
index 6fe946f..df3627a 100644 (file)
@@ -10,9 +10,15 @@ use DBIx::Class::ResultSource::RowParser::Util qw(
   assemble_simple_parser
   assemble_collapsing_parser
 );
+use DBIx::Class::_Util qw( DUMMY_ALIASPAIR dbic_internal_try dbic_internal_catch );
 
 use DBIx::Class::Carp;
 
+# FIXME - this should go away
+# instead Carp::Skip should export usable keywords or something like that
+my $unique_carper;
+BEGIN { $unique_carper = \&carp_unique }
+
 use namespace::clean;
 
 # Accepts a prefetch map (one or more relationships for the current source),
@@ -186,11 +192,28 @@ sub _resolve_collapse {
       is_single => ( $inf->{attrs}{accessor} && $inf->{attrs}{accessor} ne 'multi' ),
       is_inner => ( ( $inf->{attrs}{join_type} || '' ) !~ /^left/i),
       rsrc => $self->related_source($rel),
-      fk_map => $self->_resolve_relationship_condition(
-        rel_name => $rel,
-        self_alias => "\xFE", # irrelevant
-        foreign_alias => "\xFF", # irrelevant
-      )->{identity_map},
+      fk_map => (
+        dbic_internal_try {
+          $self->resolve_relationship_condition(
+            rel_name => $rel,
+
+            # an API where these are optional would be too cumbersome,
+            # instead always pass in some dummy values
+            DUMMY_ALIASPAIR,
+          )->{identity_map},
+        }
+        dbic_internal_catch {
+
+          $unique_carper->(
+            "Resolution of relationship '$rel' failed unexpectedly, "
+          . 'please relay the following error and seek assistance via '
+          . DBIx::Class::_ENV_::HELP_URL . ". Encountered error: $_"
+          );
+
+          # RV
+          +{}
+        }
+      ),
     };
   }
 
@@ -454,12 +477,15 @@ sub _resolve_collapse {
 
         is_single => $relinfo->{$rel}{is_single},
 
-        # if there is at least one *inner* reverse relationship which is HASH-based (equality only)
+        # if there is at least one *inner* reverse relationship ( meaning identity-only )
         # we can safely assume that the child can not exist without us
-        rev_rel_is_optional => ( grep
-          { ref $_->{cond} eq 'HASH' and ($_->{attrs}{join_type}||'') !~ /^left/i }
-          values %{ $self->reverse_relationship_info($rel) },
-        ) ? 0 : 1,
+        rev_rel_is_optional => (
+          ( grep {
+            ($_->{attrs}{join_type}||'') !~ /^left/i
+          } values %{ $self->reverse_relationship_info($rel) } )
+            ? 0
+            : 1
+        ),
 
         # if this is a 1:1 our own collapser can be used as a collapse-map
         # (regardless of left or not)
index 4188d10..cc66d74 100644 (file)
@@ -6,7 +6,10 @@ use warnings;
 use base qw/DBIx::Class/;
 
 use Scalar::Util 'blessed';
-use DBIx::Class::_Util qw( dbic_internal_try fail_on_internal_call );
+use DBIx::Class::_Util qw(
+  dbic_internal_try fail_on_internal_call
+  DUMMY_ALIASPAIR
+);
 use DBIx::Class::Carp;
 use SQL::Abstract qw( is_literal_value is_plain_value );
 
@@ -1190,14 +1193,15 @@ sub copy {
 
     $copied->{$_->ID}++ or $_->copy(
 
-      $foreign_vals ||= $rsrc->_resolve_relationship_condition(
-        infer_values_based_on => {},
+      $foreign_vals ||= $rsrc->resolve_relationship_condition(
+        require_join_free_values => 1,
         rel_name => $rel_name,
         self_result_object => $new,
 
-        self_alias => "\xFE", # irrelevant
-        foreign_alias => "\xFF", # irrelevant,
-      )->{inferred_values}
+        # an API where these are optional would be too cumbersome,
+        # instead always pass in some dummy values
+        DUMMY_ALIASPAIR,
+      )->{join_free_values}
 
     ) for $self->related_resultset($rel_name)->all;
   }
index ea69e07..6557f2e 100644 (file)
@@ -191,7 +191,7 @@ sub _assert_bindval_matches_bindtype () { 1 };
 
 # poor man's de-qualifier
 sub _quote {
-  $_[0]->next::method( ( $_[0]{_dequalify_idents} and ! ref $_[1] )
+  $_[0]->next::method( ( $_[0]{_dequalify_idents} and defined $_[1] and ! ref $_[1] )
     ? $_[1] =~ / ([^\.]+) $ /x
     : $_[1]
   );
index f029e24..430cc2b 100644 (file)
@@ -346,7 +346,7 @@ sub _normalize_cond_unroll_pairs {
       if (ref $rhs eq 'HASH' and ! keys %$rhs) {
         # FIXME - SQLA seems to be doing... nothing...?
       }
-      # normalize top level -ident, for saner extract_fixed_condition_columns code
+      # normalize top level -ident, for saner extract_equality_conditions() code
       elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-ident}) {
         push @conds, { $lhs => { '=', $rhs } };
       }
index 99a895e..7be4202 100644 (file)
@@ -11,6 +11,7 @@ use DBIx::Class::Carp;
 use Scalar::Util qw/refaddr weaken reftype blessed/;
 use Context::Preserve 'preserve_context';
 use SQL::Abstract qw(is_plain_value is_literal_value);
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use DBIx::Class::_Util qw(
   quote_sub perlstring serialize dump_value
   dbic_internal_try dbic_internal_catch
@@ -1775,7 +1776,8 @@ sub _resolve_bindattrs {
   my $resolve_bindinfo = sub {
     #my $infohash = shift;
 
-    $colinfos ||= { %{ $self->_resolve_column_info($ident) } };
+    # shallow copy to preempt autoviv
+    $colinfos ||= { %{ fromspec_columns_info($ident) } };
 
     my $ret;
     if (my $col = $_[0]->{dbic_colname}) {
@@ -2642,8 +2644,6 @@ sub _select_args {
   $orig_attrs->{_last_sqlmaker_alias_map} = $attrs->{_aliastypes};
 
 ###
-  #   my $alias2source = $self->_resolve_ident_sources ($ident);
-  #
   # This would be the point to deflate anything found in $attrs->{where}
   # (and leave $attrs->{bind} intact). Problem is - inflators historically
   # expect a result object. And all we have is a resultsource (it is trivial
index c7cb5c3..fbcd0ea 100644 (file)
@@ -2,12 +2,15 @@ package DBIx::Class::Storage::DBI::ADO::MS_Jet;
 
 use strict;
 use warnings;
+
 use base qw/
   DBIx::Class::Storage::DBI::ADO
   DBIx::Class::Storage::DBI::ACCESS
 /;
 use mro 'c3';
+
 use DBIx::Class::Storage::DBI::ADO::CursorUtils '_normalize_guids';
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use namespace::clean;
 
 __PACKAGE__->cursor_class('DBIx::Class::Storage::DBI::ADO::MS_Jet::Cursor');
@@ -104,7 +107,7 @@ sub select_single {
   return @row unless
     $self->cursor_class->isa('DBIx::Class::Storage::DBI::ADO::MS_Jet::Cursor');
 
-  my $col_infos = $self->_resolve_column_info($ident);
+  my $col_infos = fromspec_columns_info($ident);
 
   _normalize_guids($select, $col_infos, \@row, $self);
 
index 8b1a782..89ab579 100644 (file)
@@ -4,7 +4,9 @@ use strict;
 use warnings;
 use base 'DBIx::Class::Storage::DBI::Cursor';
 use mro 'c3';
+
 use DBIx::Class::Storage::DBI::ADO::CursorUtils '_normalize_guids';
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use namespace::clean;
 
 =head1 NAME
@@ -41,7 +43,7 @@ sub next {
 
   _normalize_guids(
     $self->args->[1],
-    $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]),
+    $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]),
     \@row,
     $self->storage
   );
@@ -56,7 +58,7 @@ sub all {
 
   _normalize_guids(
     $self->args->[1],
-    $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]),
+    $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]),
     $_,
     $self->storage
   ) for @rows;
index ac42a1e..33a3e13 100644 (file)
@@ -8,8 +8,10 @@ use base qw/
   DBIx::Class::Storage::DBI::MSSQL
 /;
 use mro 'c3';
+
 use DBIx::Class::Carp;
 use DBIx::Class::Storage::DBI::ADO::CursorUtils qw/_normalize_guids _strip_trailing_binary_nulls/;
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use namespace::clean;
 
 __PACKAGE__->cursor_class(
@@ -140,7 +142,7 @@ sub select_single {
     'DBIx::Class::Storage::DBI::ADO::Microsoft_SQL_Server::Cursor'
   );
 
-  my $col_infos = $self->_resolve_column_info($ident);
+  my $col_infos = fromspec_columns_info($ident);
 
   _normalize_guids($select, $col_infos, \@row, $self);
 
index 6253ee6..525526b 100644 (file)
@@ -2,9 +2,12 @@ package DBIx::Class::Storage::DBI::ADO::Microsoft_SQL_Server::Cursor;
 
 use strict;
 use warnings;
+
 use base 'DBIx::Class::Storage::DBI::Cursor';
 use mro 'c3';
+
 use DBIx::Class::Storage::DBI::ADO::CursorUtils qw/_normalize_guids _strip_trailing_binary_nulls/;
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
 use namespace::clean;
 
 =head1 NAME
@@ -42,7 +45,7 @@ sub next {
 
   my @row = $self->next::method(@_);
 
-  $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]);
+  $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]);
 
   _normalize_guids(
     $self->args->[1],
@@ -66,7 +69,7 @@ sub all {
 
   my @rows = $self->next::method(@_);
 
-  $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]);
+  $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]);
 
   for (@rows) {
     _normalize_guids(
index e9bc102..9cb8306 100644 (file)
@@ -4,6 +4,9 @@ use strict;
 use warnings;
 use base qw/DBIx::Class::Storage::DBI::UniqueIdentifier/;
 use mro 'c3';
+use DBIx::Class::_Util 'dbic_internal_try';
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
+use namespace::clean;
 
 __PACKAGE__->mk_group_accessors(simple => qw/_identity/);
 __PACKAGE__->sql_limit_dialect ('RowNumberOver');
@@ -110,7 +113,7 @@ sub select_single {
 
   my ($ident, $select) = @_;
 
-  my $col_info = $self->_resolve_column_info($ident);
+  my $col_info = fromspec_columns_info($ident);
 
   for my $select_idx (0..$#$select) {
     my $selected = $select->[$select_idx];
index a341b20..8fb08a9 100644 (file)
@@ -5,6 +5,9 @@ use warnings;
 use base 'DBIx::Class::Storage::DBI::Cursor';
 use mro 'c3';
 
+use DBIx::Class::ResultSource::FromSpec::Util 'fromspec_columns_info';
+use namespace::clean;
+
 =head1 NAME
 
 DBIx::Class::Storage::DBI::SQLAnywhere::Cursor - GUID Support for SQL Anywhere
@@ -61,7 +64,7 @@ sub next {
 
   $unpack_guids->(
     $self->args->[1],
-    $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]),
+    $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]),
     \@row,
     $self->storage
   );
@@ -76,7 +79,7 @@ sub all {
 
   $unpack_guids->(
     $self->args->[1],
-    $self->{_colinfos} ||= $self->storage->_resolve_column_info($self->args->[0]),
+    $self->{_colinfos} ||= fromspec_columns_info($self->args->[0]),
     $_,
     $self->storage
   ) for @rows;
index 317dbd8..b85fa78 100644 (file)
@@ -33,6 +33,10 @@ use DBIx::Class::_Util qw(
   dump_value fail_on_internal_call
 );
 use DBIx::Class::SQLMaker::Util 'extract_equality_conditions';
+use DBIx::Class::ResultSource::FromSpec::Util qw(
+  fromspec_columns_info
+  find_join_path_to_alias
+);
 use DBIx::Class::Carp;
 use namespace::clean;
 
@@ -167,7 +171,7 @@ sub _adjust_select_args_for_complex_prefetch {
     unless $root_node;
 
   # use the heavy duty resolver to take care of aliased/nonaliased naming
-  my $colinfo = $self->_resolve_column_info($inner_attrs->{from});
+  my $colinfo = fromspec_columns_info($inner_attrs->{from});
   my $selected_root_columns;
 
   for my $i (0 .. $#{$outer_attrs->{select}}) {
@@ -444,7 +448,7 @@ sub _resolve_aliastypes_from_select_args {
   }
 
   # get a column to source/alias map (including unambiguous unqualified ones)
-  my $colinfo = $self->_resolve_column_info ($attrs->{from});
+  my $colinfo = fromspec_columns_info($attrs->{from});
 
   # set up a botched SQLA
   my $sql_maker = $self->sql_maker;
@@ -633,7 +637,7 @@ sub _resolve_aliastypes_from_select_args {
 sub _group_over_selection {
   my ($self, $attrs) = @_;
 
-  my $colinfos = $self->_resolve_column_info ($attrs->{from});
+  my $colinfos = fromspec_columns_info($attrs->{from});
 
   my (@group_by, %group_index);
 
@@ -769,181 +773,6 @@ sub _minmax_operator_for_datatype {
   $_[2] ? 'MAX' : 'MIN';
 }
 
-sub _resolve_ident_sources {
-  my ($self, $ident) = @_;
-
-  my $alias2source = {};
-
-  # the reason this is so contrived is that $ident may be a {from}
-  # structure, specifying multiple tables to join
-  if ( blessed $ident && $ident->isa("DBIx::Class::ResultSource") ) {
-    # this is compat mode for insert/update/delete which do not deal with aliases
-    $alias2source->{me} = $ident;
-  }
-  elsif (ref $ident eq 'ARRAY') {
-
-    for (@$ident) {
-      my $tabinfo;
-      if (ref $_ eq 'HASH') {
-        $tabinfo = $_;
-      }
-      if (ref $_ eq 'ARRAY' and ref $_->[0] eq 'HASH') {
-        $tabinfo = $_->[0];
-      }
-
-      $alias2source->{$tabinfo->{-alias}} = $tabinfo->{-rsrc}
-        if ($tabinfo->{-rsrc});
-    }
-  }
-
-  return $alias2source;
-}
-
-# Takes $ident, \@column_names
-#
-# returns { $column_name => \%column_info, ... }
-# also note: this adds -result_source => $rsrc to the column info
-#
-# If no columns_names are supplied returns info about *all* columns
-# for all sources
-sub _resolve_column_info {
-  my ($self, $ident, $colnames) = @_;
-
-  return {} if $colnames and ! @$colnames;
-
-  my $sources = $self->_resolve_ident_sources($ident);
-
-  $_ = { rsrc => $_, colinfos => $_->columns_info }
-    for values %$sources;
-
-  my (%seen_cols, @auto_colnames);
-
-  # compile a global list of column names, to be able to properly
-  # disambiguate unqualified column names (if at all possible)
-  for my $alias (keys %$sources) {
-    (
-      ++$seen_cols{$_}{$alias}
-        and
-      ! $colnames
-        and
-      push @auto_colnames, "$alias.$_"
-    ) for keys %{ $sources->{$alias}{colinfos} };
-  }
-
-  $colnames ||= [
-    @auto_colnames,
-    ( grep { keys %{$seen_cols{$_}} == 1 } keys %seen_cols ),
-  ];
-
-  my %return;
-  for (@$colnames) {
-    my ($colname, $source_alias) = reverse split /\./, $_;
-
-    my $assumed_alias =
-      $source_alias
-        ||
-      # if the column was seen exactly once - we know which rsrc it came from
-      (
-        $seen_cols{$colname}
-          and
-        keys %{$seen_cols{$colname}} == 1
-          and
-        ( %{$seen_cols{$colname}} )[0]
-      )
-        ||
-      next
-    ;
-
-    $self->throw_exception(
-      "No such column '$colname' on source " . $sources->{$assumed_alias}{rsrc}->source_name
-    ) unless $seen_cols{$colname}{$assumed_alias};
-
-    $return{$_} = {
-      %{ $sources->{$assumed_alias}{colinfos}{$colname} },
-      -result_source => $sources->{$assumed_alias}{rsrc},
-      -source_alias => $assumed_alias,
-      -fq_colname => "$assumed_alias.$colname",
-      -colname => $colname,
-    };
-
-    $return{"$assumed_alias.$colname"} = $return{$_}
-      unless $source_alias;
-  }
-
-  return \%return;
-}
-
-# 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.
-#
-sub _inner_join_to_node {
-  my ($self, $from, $alias) = @_;
-
-  my $switch_branch = $self->_find_join_path_to_node($from, $alias);
-
-  return $from unless @{$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 = ($from->[0]);
-  my $sw_idx = { map { (values %$_), 1 } @$switch_branch }; #there's one k/v per join-path
-
-  for my $j (@{$from}[1 .. $#$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;
-    }
-  }
-
-  return \@new_from;
-}
-
-sub _find_join_path_to_node {
-  my ($self, $from, $target_alias) = @_;
-
-  # subqueries and other oddness are naturally not supported
-  return undef if (
-    ref $from ne 'ARRAY'
-      ||
-    ref $from->[0] ne 'HASH'
-      ||
-    ! defined $from->[0]{-alias}
-  );
-
-  # no path - the head is the alias
-  return [] if $from->[0]{-alias} eq $target_alias;
-
-  for my $i (1 .. $#$from) {
-    return $from->[$i][0]{-join_path} if ( ($from->[$i][0]{-alias}||'') eq $target_alias );
-  }
-
-  # something else went quite wrong
-  return undef;
-}
-
 sub _extract_order_criteria {
   my ($self, $order_by, $sql_maker) = @_;
 
@@ -997,7 +826,7 @@ sub _order_by_is_stable {
     ( $where ? keys %{ extract_equality_conditions( $where ) } : () ),
   ) or return 0;
 
-  my $colinfo = $self->_resolve_column_info($ident, \@cols);
+  my $colinfo = fromspec_columns_info($ident, \@cols);
 
   return keys %$colinfo
     ? $self->_columns_comprise_identifying_set( $colinfo,  \@cols )
@@ -1027,7 +856,7 @@ sub _columns_comprise_identifying_set {
 sub _extract_colinfo_of_stable_main_source_order_by_portion {
   my ($self, $attrs) = @_;
 
-  my $nodes = $self->_find_join_path_to_node($attrs->{from}, $attrs->{alias});
+  my $nodes = find_join_path_to_alias($attrs->{from}, $attrs->{alias});
 
   return unless defined $nodes;
 
@@ -1042,7 +871,7 @@ sub _extract_colinfo_of_stable_main_source_order_by_portion {
     map { values %$_ } @$nodes,
   ) };
 
-  my $colinfos = $self->_resolve_column_info($attrs->{from});
+  my $colinfos = fromspec_columns_info($attrs->{from});
 
   my ($colinfos_to_return, $seen_main_src_cols);
 
@@ -1083,6 +912,20 @@ sub _extract_colinfo_of_stable_main_source_order_by_portion {
   ]) ? $colinfos_to_return : ();
 }
 
+sub _resolve_column_info :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique("_resolve_column_info() is deprecated, ask on IRC for a better alternative");
+
+  fromspec_columns_info( @_[1,2] );
+}
+
+sub _find_join_path_to_node :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique("_find_join_path_to_node() is deprecated, ask on IRC for a better alternative");
+
+  find_join_path_to_alias( @_[1,2] );
+}
+
 sub _collapse_cond :DBIC_method_is_indirect_sugar {
   DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
   carp_unique("_collapse_cond() is deprecated, ask on IRC for a better alternative");
@@ -1099,4 +942,18 @@ sub _extract_fixed_condition_columns :DBIC_method_is_indirect_sugar {
   extract_equality_conditions(@_);
 }
 
+sub _resolve_ident_sources :DBIC_method_is_indirect_sugar {
+  DBIx::Class::Exception->throw(
+    '_resolve_ident_sources() has been removed with no replacement, '
+  . 'ask for advice on IRC if this affected you'
+  );
+}
+
+sub _inner_join_to_node :DBIC_method_is_indirect_sugar {
+  DBIx::Class::Exception->throw(
+    '_inner_join_to_node() has been removed with no replacement, '
+  . 'ask for advice on IRC if this affected you'
+  );
+}
+
 1;
index b8f0b06..29b196d 100644 (file)
@@ -52,6 +52,7 @@ BEGIN {
         DBIC_ASSERT_NO_INTERNAL_INDIRECT_CALLS
         DBIC_ASSERT_NO_ERRONEOUS_METAINSTANCE_USE
         DBIC_ASSERT_NO_FAILING_SANITY_CHECKS
+        DBIC_ASSERT_NO_INCONSISTENT_RELATIONSHIP_RESOLUTION
         DBIC_STRESSTEST_UTF8_UPGRADE_GENERATED_COLLAPSER_SOURCE
         DBIC_STRESSTEST_COLUMN_INFO_UNAWARE_STORAGE
       )
@@ -203,13 +204,18 @@ our @EXPORT_OK = qw(
   scope_guard detected_reinvoked_destructor emit_loud_diag
   true false
   is_exception dbic_internal_try dbic_internal_catch visit_namespaces
-  quote_sub qsub perlstring serialize deep_clone dump_value uniq
+  quote_sub qsub perlstring serialize deep_clone dump_value uniq bag_eq
   parent_dir mkdir_p
-  UNRESOLVABLE_CONDITION
+  UNRESOLVABLE_CONDITION DUMMY_ALIASPAIR
 );
 
 use constant UNRESOLVABLE_CONDITION => \ '1 = 0';
 
+use constant DUMMY_ALIASPAIR => (
+  foreign_alias => "!!!\xFF()!!!_DUMMY_FOREIGN_ALIAS_SHOULD_NEVER_BE_SEEN_IN_USE_!!!()\xFF!!!",
+  self_alias => "!!!\xFE()!!!_DUMMY_SELF_ALIAS_SHOULD_NEVER_BE_SEEN_IN_USE_!!!()\xFE!!!",
+);
+
 # Override forcing no_defer, and adding naming consistency checks
 our %refs_closed_over_by_quote_sub_installed_crefs;
 sub quote_sub {
@@ -381,6 +387,34 @@ sub uniq {
   ) } @_;
 }
 
+sub bag_eq ($$) {
+  croak "bag_eq() requiress two arrayrefs as arguments" if (
+    ref($_[0]) ne 'ARRAY'
+      or
+    ref($_[1]) ne 'ARRAY'
+  );
+
+  return '' unless @{$_[0]} == @{$_[1]};
+
+  my( %seen, $numeric_preserving_copy );
+
+  ( defined $_
+    ? $seen{'value' . ( $numeric_preserving_copy = $_ )}++
+    : $seen{'undef'}++
+  ) for @{$_[0]};
+
+  ( defined $_
+    ? $seen{'value' . ( $numeric_preserving_copy = $_ )}--
+    : $seen{'undef'}--
+  ) for @{$_[1]};
+
+  return (
+    (grep { $_ } values %seen)
+      ? ''
+      : 1
+  );
+}
+
 my $dd_obj;
 sub dump_value ($) {
   local $Data::Dumper::Indent = 1
index 623171e..14812ac 100644 (file)
@@ -15,7 +15,7 @@ $DEBUG = 0 unless defined $DEBUG;
 use Exporter;
 use SQL::Translator::Utils qw(debug normalize_name);
 use DBIx::Class::Carp qw/^SQL::Translator|^DBIx::Class|^Try::Tiny/;
-use DBIx::Class::_Util qw( dbic_internal_try dbic_internal_catch );
+use DBIx::Class::_Util qw( dbic_internal_try dbic_internal_catch bag_eq );
 use Class::C3::Componentised;
 use Scalar::Util 'blessed';
 use namespace::clean;
@@ -155,13 +155,11 @@ sub parse {
 
         my %unique_constraints = $source->unique_constraints;
         foreach my $uniq (sort keys %unique_constraints) {
-            if (!$source->_compare_relationship_keys($unique_constraints{$uniq}, \@primary)) {
-                $table->add_constraint(
-                            type             => 'unique',
-                            name             => $uniq,
-                            fields           => $unique_constraints{$uniq}
-                );
-            }
+            $table->add_constraint(
+                type             => 'unique',
+                name             => $uniq,
+                fields           => $unique_constraints{$uniq}
+            ) unless bag_eq( \@primary, $unique_constraints{$uniq} );
         }
 
         my @rels = $source->relationships();
@@ -177,6 +175,11 @@ sub parse {
             my $rel_info = $source->relationship_info($rel);
 
             # Ignore any rel cond that isn't a straight hash
+            #
+            # FIXME - this can be done *WAY* better via the recolcond resolver
+            # but no time to think through the implications for deploy() at
+            # the moment. Grep for {identity_map_matches_condition} for ideas
+            # how to improve this, and the /^\w+\.(\w+)$/ crap below
             next unless ref $rel_info->{cond} eq 'HASH';
 
             my $relsource = dbic_internal_try { $source->related_source($rel) };
@@ -227,7 +230,7 @@ sub parse {
             # this is supposed to indicate a has_one/might_have...
             # where's the introspection!!?? :)
             else {
-                $fk_constraint = not $source->_compare_relationship_keys(\@keys, \@primary);
+                $fk_constraint = ! bag_eq( \@keys, \@primary );
             }
 
 
index 6d47c12..abad170 100644 (file)
@@ -118,7 +118,7 @@ sub fail_with_bad_object {
         NumExplodingSheep => 23
       }
     );
-  } qr/isn't a Director/;
+  } qr/is not a column on related source 'Director'/;
 }
 
 package Foo;
index a7b069c..6304b2c 100644 (file)
@@ -110,7 +110,7 @@ is(
         Rating            => 'R',
         NumExplodingSheep => 23
       });
-  } qr/isn't a Director/, "Can't have film as codirector";
+  } qr/is not a column on related source 'Director'/, "Can't have film as codirector";
   is $fail, undef, "We didn't get anything";
 
   my $tastes_bad = YA::Film->create({
index de6afd7..6ebf94f 100644 (file)
@@ -266,7 +266,11 @@ is($undir_maps->count, 1, 'found 1 undirected map for artist 2');
 {
   my $artist_to_mangle = $schema->resultset('Artist')->find(2);
 
-  $artist_to_mangle->set_from_related( artist_undirected_maps => { id1 => 42 } );
+  throws_ok {
+    $artist_to_mangle->set_from_related( artist_undirected_maps => { id1 => 42 } )
+  } qr/\QUnable to complete value inferrence - relationship 'artist_undirected_maps' on source 'Artist' results in expression(s) instead of definitive values: ( artistid = ? OR artistid IS NULL )/,
+    'Expected exception on unresovable set_from_related'
+  ;
 
   ok( ! $artist_to_mangle->is_changed, 'Unresolvable set_from_related did not alter object' );
 
index 32c8cf8..2646505 100644 (file)
@@ -162,7 +162,7 @@ lives_ok {
 # try to create_related a 80s cd
 throws_ok {
   $artist->create_related('cds_80s', { title => 'related creation 1' });
-} qr/\QUnable to complete value inferrence - custom relationship 'cds_80s' on source 'Artist' returns conditions instead of values for column(s): 'year'/,
+} qr/\QUnable to complete value inferrence - relationship 'cds_80s' on source 'Artist' results in expression(s) instead of definitive values: ( year < ? AND year > ? )/,
 'Create failed - complex cond';
 
 # now supply an explicit arg overwriting the ambiguous cond
index aacd59c..93b3c16 100644 (file)
@@ -18,6 +18,17 @@ my ($ROWS, $OFFSET) = (
 
 my $schema = DBICTest->init_schema();
 
+$schema->is_executed_sql_bind(
+  sub { $schema->resultset('Artist')->find( Math::BigInt->new(42) ) },
+  [
+    [
+      'SELECT me.artistid, me.name, me.rank, me.charfield FROM artist me WHERE me.artistid = ?',
+      [ { dbic_colname => "me.artistid", sqlt_datatype => "integer" }
+        => Math::BigInt->new(42) ],
+    ]
+  ]
+);
+
 my $rs = $schema->resultset('CD')->search({ -and => [
   'me.artist' => { '!=', '666' },
   'me.artist' => { '!=', \[ '?', [ _ne => 'bar' ] ] },
diff --git a/xt/extra/diagnostics/find_via_unsupported_rel.t b/xt/extra/diagnostics/find_via_unsupported_rel.t
new file mode 100644 (file)
index 0000000..10328e2
--- /dev/null
@@ -0,0 +1,31 @@
+BEGIN { do "./t/lib/ANFANG.pm" or die ( $@ || $! ) }
+
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+
+use DBICTest;
+
+my $schema = DBICTest->init_schema( no_deploy => 1 );
+
+my $artist = $schema->resultset('Artist')->new_result({ artistid => 1 });
+
+throws_ok {
+  $schema->resultset('ArtistUndirectedMap')->find({
+    mapped_artists => $artist,
+  });
+} qr/\QUnable to complete value inferrence - relationship 'mapped_artists' on source 'ArtistUndirectedMap' results in expression(s) instead of definitive values: ( id1 = ? OR id2 = ? )/,
+  'proper exception on OR relationship inferrence'
+;
+
+throws_ok {
+  $schema->resultset('Artwork_to_Artist')->find({
+    artist_limited_rank_opaque => $artist
+  })
+} qr/\QRelationship 'artist_limited_rank_opaque' on source 'Artwork_to_Artist' does not resolve to a 'foreign_values'-based reversed-join-free condition fragment/,
+  'proper exception on ipaque custom cond'
+;
+
+done_testing;
@@ -6,10 +6,9 @@ use warnings;
 use Test::More;
 use Test::Exception;
 
-
 use DBICTest;
 
-my $schema = DBICTest->init_schema();
+my $schema = DBICTest->init_schema( no_deploy => 1 );
 
 for (
   { year => [1,2] },
@@ -18,7 +17,7 @@ for (
   { -and => [ year => 1, year => 2 ] },
 ) {
   throws_ok {
-    $schema->source('Track')->_resolve_relationship_condition(
+    $schema->source('Track')->resolve_relationship_condition(
       rel_name => 'cd_cref_cond',
       self_alias => 'me',
       foreign_alias => 'cd',
@@ -27,7 +26,9 @@ for (
   } qr/
     \Qis not a column on related source 'CD'\E
       |
-    \QValue supplied for '...{foreign_values}{year}' is not a direct equivalence expression\E
+    \Qsupplied value for foreign column 'year' is not a direct equivalence expression\E
+      |
+    \QThe key '-\E \w+ \Q' supplied as part of 'foreign_values' during relationship resolution must be a column name, not a function\E
   /x;
 }
 
index 89e2b54..eb25530 100644 (file)
@@ -80,6 +80,7 @@ my $skip_idx = { map { $_ => 1 } (
   # utility classes, not part of the inheritance chain
   'DBIx::Class::Optional::Dependencies',
   'DBIx::Class::ResultSource::RowParser::Util',
+  'DBIx::Class::ResultSource::FromSpec::Util',
   'DBIx::Class::SQLMaker::Util',
   'DBIx::Class::_Util',
 ) };