Even more robust _resolve_condition shim - guess rel_name when needed
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSource.pm
index 818341b..fb4c27d 100644 (file)
@@ -8,8 +8,8 @@ use base qw/DBIx::Class::ResultSource::RowParser DBIx::Class/;
 use DBIx::Class::ResultSet;
 use DBIx::Class::ResultSourceHandle;
 
-use DBIx::Class::Exception;
 use DBIx::Class::Carp;
+use DBIx::Class::_Util 'is_literal_value';
 use Devel::GlobalDestruction;
 use Try::Tiny;
 use List::Util 'first';
@@ -85,7 +85,7 @@ created, see L<DBIx::Class::ResultSource::View> for full details.
 =head2 Finding result source objects
 
 As mentioned above, a result source instance is created and stored for
-you when you define a L<Result Class|DBIx::Class::Manual::Glossary/Result Class>.
+you when you define a L<result class|DBIx::Class::Manual::Glossary/Result class>.
 
 You can retrieve the result source at runtime in the following ways:
 
@@ -95,9 +95,9 @@ You can retrieve the result source at runtime in the following ways:
 
    $schema->source($source_name);
 
-=item From a Row object:
+=item From a Result object:
 
-   $row->result_source;
+   $result->result_source;
 
 =item From a ResultSet object:
 
@@ -134,7 +134,7 @@ sub new {
 
 =item Arguments: @columns
 
-=item Return value: The ResultSource object
+=item Return Value: L<$result_source|/new>
 
 =back
 
@@ -147,7 +147,7 @@ pairs, uses the hashref as the L</column_info> for that column. Repeated
 calls of this method will add more columns, not replace them.
 
 The column names given will be created as accessor methods on your
-L<DBIx::Class::Row> objects. You can change the name of the accessor
+L<Result|DBIx::Class::Manual::ResultClass> objects. You can change the name of the accessor
 by supplying an L</accessor> in the column_info hash.
 
 If a column name beginning with a plus sign ('+col1') is provided, the
@@ -202,7 +202,7 @@ schema, see L<DBIx::Class::Schema/deploy>.
 
    { is_nullable => 1 }
 
-Set this to a true value for a columns that is allowed to contain NULL
+Set this to a true value for a column that is allowed to contain NULL
 values, default is false. This is currently only used to create tables
 from your schema, see L<DBIx::Class::Schema/deploy>.
 
@@ -300,7 +300,7 @@ L<SQL::Translator::Producer::MySQL>.
 
 =item Arguments: $colname, \%columninfo?
 
-=item Return value: 1/0 (true/false)
+=item Return Value: 1/0 (true/false)
 
 =back
 
@@ -344,7 +344,7 @@ sub add_column { shift->add_columns(@_); } # DO NOT CHANGE THIS TO GLOB
 
 =item Arguments: $colname
 
-=item Return value: 1/0 (true/false)
+=item Return Value: 1/0 (true/false)
 
 =back
 
@@ -365,7 +365,7 @@ sub has_column {
 
 =item Arguments: $colname
 
-=item Return value: Hashref of info
+=item Return Value: Hashref of info
 
 =back
 
@@ -413,9 +413,9 @@ sub column_info {
 
 =over
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: Ordered list of column names
+=item Return Value: Ordered list of column names
 
 =back
 
@@ -439,7 +439,7 @@ sub columns {
 
 =item Arguments: \@colnames ?
 
-=item Return value: Hashref of column name/info pairs
+=item Return Value: Hashref of column name/info pairs
 
 =back
 
@@ -493,9 +493,9 @@ sub columns_info {
       }
       else {
         $self->throw_exception( sprintf (
-          "No such column '%s' on source %s",
+          "No such column '%s' on source '%s'",
           $_,
-          $self->source_name,
+          $self->source_name || $self->name || 'Unknown source...?',
         ));
       }
     }
@@ -513,7 +513,7 @@ sub columns_info {
 
 =item Arguments: @colnames
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -531,7 +531,7 @@ broken result source.
 
 =item Arguments: $colname
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -569,7 +569,7 @@ sub remove_column { shift->remove_columns(@_); } # DO NOT CHANGE THIS TO GLOB
 
 =item Arguments: @cols
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -589,11 +589,18 @@ for more info.
 
 sub set_primary_key {
   my ($self, @cols) = @_;
-  # check if primary key columns are valid columns
-  foreach my $col (@cols) {
-    $self->throw_exception("No such column $col on table " . $self->name)
-      unless $self->has_column($col);
+
+  my $colinfo = $self->columns_info(\@cols);
+  for my $col (@cols) {
+    carp_unique(sprintf (
+      "Primary key of source '%s' includes the column '%s' which has its "
+    . "'is_nullable' attribute set to true. This is a mistake and will cause "
+    . 'various Result-object operations to fail',
+      $self->source_name || $self->name || 'Unknown source...?',
+      $col,
+    )) if $colinfo->{$col}{is_nullable};
   }
+
   $self->_primaries(\@cols);
 
   $self->add_unique_constraint(primary => \@cols);
@@ -603,9 +610,9 @@ sub set_primary_key {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: Ordered list of primary column names
+=item Return Value: Ordered list of primary column names
 
 =back
 
@@ -621,7 +628,7 @@ sub primary_columns {
 # a helper method that will automatically die with a descriptive message if
 # no pk is defined on the source in question. For internal use to save
 # on if @pks... boilerplate
-sub _pri_cols {
+sub _pri_cols_or_die {
   my $self = shift;
   my @pcols = $self->primary_columns
     or $self->throw_exception (sprintf(
@@ -632,6 +639,20 @@ sub _pri_cols {
   return @pcols;
 }
 
+# same as above but mandating single-column PK (used by relationship condition
+# inference)
+sub _single_pri_col_or_die {
+  my $self = shift;
+  my ($pri, @too_many) = $self->_pri_cols_or_die;
+
+  $self->throw_exception( sprintf(
+    "Operation requires a single-column primary key declared on '%s'",
+    $self->source_name || $self->result_class || $self->name || 'Unknown source...?',
+  )) if @too_many;
+  return $pri;
+}
+
+
 =head2 sequence
 
 Manually define the correct sequence for your table, to avoid the overhead
@@ -642,7 +663,7 @@ will be applied to the L</column_info> of each L<primary_key|/set_primary_key>
 
 =item Arguments: $sequence_name
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -665,7 +686,7 @@ sub sequence {
 
 =item Arguments: $name?, \@colnames
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -731,7 +752,7 @@ sub add_unique_constraint {
 
 =item Arguments: @constraints
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -783,7 +804,7 @@ sub add_unique_constraints {
 
 =item Arguments: \@colnames
 
-=item Return value: Constraint name
+=item Return Value: Constraint name
 
 =back
 
@@ -817,9 +838,9 @@ sub name_unique_constraint {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: Hash of unique constraint data
+=item Return Value: Hash of unique constraint data
 
 =back
 
@@ -841,9 +862,9 @@ sub unique_constraints {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: Unique constraint names
+=item Return Value: Unique constraint names
 
 =back
 
@@ -867,7 +888,7 @@ sub unique_constraint_names {
 
 =item Arguments: $constraintname
 
-=item Return value: List of constraint columns
+=item Return Value: List of constraint columns
 
 =back
 
@@ -895,7 +916,7 @@ sub unique_constraint_columns {
 
 =item Arguments: $callback_name | \&callback_code
 
-=item Return value: $callback_name | \&callback_code
+=item Return Value: $callback_name | \&callback_code
 
 =back
 
@@ -962,13 +983,39 @@ sub _invoke_sqlt_deploy_hook {
   }
 }
 
+=head2 result_class
+
+=over 4
+
+=item Arguments: $classname
+
+=item Return Value: $classname
+
+=back
+
+ use My::Schema::ResultClass::Inflator;
+ ...
+
+ use My::Schema::Artist;
+ ...
+ __PACKAGE__->result_class('My::Schema::ResultClass::Inflator');
+
+Set the default result class for this source. You can use this to create
+and use your own result inflator. See L<DBIx::Class::ResultSet/result_class>
+for more details.
+
+Please note that setting this to something like
+L<DBIx::Class::ResultClass::HashRefInflator> will make every result unblessed
+and make life more difficult.  Inflators like those are better suited to
+temporary usage via L<DBIx::Class::ResultSet/result_class>.
+
 =head2 resultset
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: $resultset
+=item Return Value: L<$resultset|DBIx::Class::ResultSet>
 
 =back
 
@@ -985,7 +1032,7 @@ but is cached from then on unless resultset_class changes.
 
 =item Arguments: $classname
 
-=item Return value: $classname
+=item Return Value: $classname
 
 =back
 
@@ -1009,9 +1056,9 @@ exists.
 
 =over 4
 
-=item Arguments: \%attrs
+=item Arguments: L<\%attrs|DBIx::Class::ResultSet/ATTRIBUTES>
 
-=item Return value: \%attrs
+=item Return Value: L<\%attrs|DBIx::Class::ResultSet/ATTRIBUTES>
 
 =back
 
@@ -1022,8 +1069,35 @@ exists.
   $source->resultset_attributes({ order_by => [ 'id' ] });
 
 Store a collection of resultset attributes, that will be set on every
-L<DBIx::Class::ResultSet> produced from this result source. For a full
-list see L<DBIx::Class::ResultSet/ATTRIBUTES>.
+L<DBIx::Class::ResultSet> produced from this result source.
+
+B<CAVEAT>: C<resultset_attributes> comes with its own set of issues and
+bugs! While C<resultset_attributes> isn't deprecated per se, its usage is
+not recommended!
+
+Since relationships use attributes to link tables together, the "default"
+attributes you set may cause unpredictable and undesired behavior.  Furthermore,
+the defaults cannot be turned off, so you are stuck with them.
+
+In most cases, what you should actually be using are project-specific methods:
+
+  package My::Schema::ResultSet::Artist;
+  use base 'DBIx::Class::ResultSet';
+  ...
+
+  # BAD IDEA!
+  #__PACKAGE__->resultset_attributes({ prefetch => 'tracks' });
+
+  # GOOD IDEA!
+  sub with_tracks { shift->search({}, { prefetch => 'tracks' }) }
+
+  # in your code
+  $schema->resultset('Artist')->with_tracks->...
+
+This gives you the flexibility of not using it when you don't need it.
+
+For more complex situations, another solution would be to use a virtual view
+via L<DBIx::Class::ResultSource::View>.
 
 =cut
 
@@ -1047,7 +1121,7 @@ sub resultset {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
 =item Result value: $name
 
@@ -1083,9 +1157,9 @@ its class name.
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: FROM clause
+=item Return Value: FROM clause
 
 =back
 
@@ -1103,9 +1177,9 @@ sub from { die 'Virtual method!' }
 
 =over 4
 
-=item Arguments: $schema
+=item Arguments: L<$schema?|DBIx::Class::Schema>
 
-=item Return value: A schema object
+=item Return Value: L<$schema|DBIx::Class::Schema>
 
 =back
 
@@ -1139,17 +1213,15 @@ sub schema {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: A Storage object
+=item Return Value: L<$storage|DBIx::Class::Storage>
 
 =back
 
   $source->storage->debug(1);
 
-Returns the storage handle for the current schema.
-
-See also: L<DBIx::Class::Storage>
+Returns the L<storage handle|DBIx::Class::Storage> for the current schema.
 
 =cut
 
@@ -1159,13 +1231,13 @@ sub storage { shift->schema->storage; }
 
 =over 4
 
-=item Arguments: $relname, $related_source_name, \%cond, [ \%attrs ]
+=item Arguments: $rel_name, $related_source_name, \%cond, \%attrs?
 
-=item Return value: 1/true if it succeeded
+=item Return Value: 1/true if it succeeded
 
 =back
 
-  $source->add_relationship('relname', 'related_source', $cond, $attrs);
+  $source->add_relationship('rel_name', 'related_source', $cond, $attrs);
 
 L<DBIx::Class::Relationship> describes a series of methods which
 create pre-defined useful types of relationships. Look there first
@@ -1285,13 +1357,13 @@ sub add_relationship {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: List of relationship names
+=item Return Value: L<@rel_names|DBIx::Class::Relationship>
 
 =back
 
-  my @relnames = $source->relationships();
+  my @rel_names = $source->relationships();
 
 Returns all relationship names for this source.
 
@@ -1305,29 +1377,29 @@ sub relationships {
 
 =over 4
 
-=item Arguments: $relname
+=item Arguments: L<$rel_name|DBIx::Class::Relationship>
 
-=item Return value: Hashref of relation data,
+=item Return Value: L<\%rel_data|DBIx::Class::Relationship::Base/add_relationship>
 
 =back
 
 Returns a hash of relationship information for the specified relationship
-name. The keys/values are as specified for L</add_relationship>.
+name. The keys/values are as specified for L<DBIx::Class::Relationship::Base/add_relationship>.
 
 =cut
 
 sub relationship_info {
-  my ($self, $rel) = @_;
-  return $self->_relationships->{$rel};
+  #my ($self, $rel) = @_;
+  return shift->_relationships->{+shift};
 }
 
 =head2 has_relationship
 
 =over 4
 
-=item Arguments: $rel
+=item Arguments: L<$rel_name|DBIx::Class::Relationship>
 
-=item Return value: 1/0 (true/false)
+=item Return Value: 1/0 (true/false)
 
 =back
 
@@ -1336,17 +1408,17 @@ Returns true if the source has a relationship of this name, false otherwise.
 =cut
 
 sub has_relationship {
-  my ($self, $rel) = @_;
-  return exists $self->_relationships->{$rel};
+  #my ($self, $rel) = @_;
+  return exists shift->_relationships->{+shift};
 }
 
 =head2 reverse_relationship_info
 
 =over 4
 
-=item Arguments: $relname
+=item Arguments: L<$rel_name|DBIx::Class::Relationship>
 
-=item Return value: Hashref of relationship data
+=item Return Value: L<\%rel_data|DBIx::Class::Relationship::Base/add_relationship>
 
 =back
 
@@ -1376,12 +1448,10 @@ sub reverse_relationship_info {
 
   my $stripped_cond = $self->__strip_relcond ($rel_info->{cond});
 
-  my $rsrc_schema_moniker = $self->source_name
-    if try { $self->schema };
+  my $registered_source_name = $self->source_name;
 
   # this may be a partial schema or something else equally esoteric
-  my $other_rsrc = try { $self->related_source($rel) }
-    or return $ret;
+  my $other_rsrc = $self->related_source($rel);
 
   # 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
@@ -1396,11 +1466,11 @@ sub reverse_relationship_info {
     my $roundtrip_rsrc = try { $other_rsrc->related_source($other_rel) }
       or next;
 
-    if ($rsrc_schema_moniker and try { $roundtrip_rsrc->schema } ) {
-      next unless $rsrc_schema_moniker eq $roundtrip_rsrc->source_name;
+    if ($registered_source_name) {
+      next if $registered_source_name ne ($roundtrip_rsrc->source_name || '')
     }
     else {
-      next unless $self->result_class eq $roundtrip_rsrc->result_class;
+      next if $self->result_class ne $roundtrip_rsrc->result_class;
     }
 
     my $other_rel_info = $other_rsrc->relationship_info($other_rel);
@@ -1550,7 +1620,7 @@ sub _resolve_join {
                   first { $rel_info->{attrs}{accessor} eq $_ } (qw/single filter/)
                 ),
                -alias => $as,
-               -relation_chain_depth => $seen->{-relation_chain_depth} || 0,
+               -relation_chain_depth => ( $seen->{-relation_chain_depth} || 0 ) + 1,
              },
              scalar $self->_resolve_condition($rel_info->{cond}, $as, $alias, $join)
           ];
@@ -1567,9 +1637,9 @@ sub pk_depends_on {
 # having already been inserted. Takes the name of the relationship and a
 # hashref of columns of the related object.
 sub _pk_depends_on {
-  my ($self, $relname, $rel_data) = @_;
+  my ($self, $rel_name, $rel_data) = @_;
 
-  my $relinfo = $self->relationship_info($relname);
+  my $relinfo = $self->relationship_info($rel_name);
 
   # don't assume things if the relationship direction is specified
   return $relinfo->{attrs}{is_foreign_key_constraint}
@@ -1584,7 +1654,7 @@ sub _pk_depends_on {
   # assume anything that references our PK probably is dependent on us
   # rather than vice versa, unless the far side is (a) defined or (b)
   # auto-increment
-  my $rel_source = $self->related_source($relname);
+  my $rel_source = $self->related_source($rel_name);
 
   foreach my $p ($self->primary_columns) {
     if (exists $keyhash->{$p}) {
@@ -1599,161 +1669,281 @@ sub _pk_depends_on {
   return 1;
 }
 
-sub resolve_condition {
-  carp 'resolve_condition is a private method, stop calling it';
-  my $self = shift;
-  $self->_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,
+#  ;
+
+#######################
+### API Design? What's that...? (a backwards compatible shim, kill me now)
+
+  my ($self, $cond, @res_args, $rel_name);
+
+  # we *SIMPLY DON'T KNOW YET* which arg is which, yay
+  ($self, $cond, $res_args[0], $res_args[1], $rel_name) = @_;
+
+  # assume that an undef is an object-like unset (set_from_related(undef))
+  my @is_objlike = map { ! defined $_ or length ref $_ } (@res_args);
+
+  # turn objlike into proper objects for saner code further down
+  for (0,1) {
+    next unless $is_objlike[$_];
+
+    if ( defined blessed $res_args[$_] ) {
+
+      # but wait - there is more!!! WHAT THE FUCK?!?!?!?!
+      if ($res_args[$_]->isa('DBIx::Class::ResultSet')) {
+        carp('Passing a resultset for relationship resolution makes no sense - invoking __gremlins__');
+        $is_objlike[$_] = 0;
+        $res_args[$_] = '__gremlins__';
+      }
+    }
+    else {
+      $res_args[$_] ||= {};
+
+      $self->throw_exception("Unsupported object-like structure encountered: $res_args[$_]")
+        unless ref $res_args[$_] eq 'HASH';
+
+      # hate everywhere
+      $res_args[$_] = $self->relationship_info($rel_name)->{source}->result_class->new($res_args[$_]);
+    }
+  }
+
+  $self->throw_exception('No practical way to resolve a relationship between two structures')
+    if $is_objlike[0] and $is_objlike[1];
+
+  my $args = {
+    condition => $cond,
+
+    # where-is-waldo block guesses relname, then further down we override it if available
+    (
+      $is_objlike[1] ? ( rel_name => $res_args[0], self_alias => $res_args[0], foreign_alias => 'me',         self_resultobj    => $res_args[1] )
+    : $is_objlike[0] ? ( rel_name => $res_args[1], self_alias => 'me',         foreign_alias => $res_args[1], foreign_resultobj => $res_args[0] )
+    :                  ( rel_name => $res_args[0], self_alias => $res_args[1], foreign_alias => $res_args[0]                                    )
+    ),
+
+    ( $rel_name ? ( rel_name => $rel_name ) : () ),
+  };
+#######################
+
+  # now it's fucking easy isn't it?!
+  my @res = $self->_resolve_relationship_condition( $args );
+
+  # FIXME - this is also insane, but just be consistent for now
+  # _resolve_relationship_condition always returns qualified cols
+  # even in the case of objects, but nothing downstream expects this
+  if (ref $res[0] eq 'HASH' and ($is_objlike[0] or $is_objlike[1]) ) {
+    $res[0] = { map
+      { ($_ =~ /\.(.+)/) => $res[0]{$_} }
+      keys %{$res[0]}
+    };
+  }
+
+  # more legacy
+  return wantarray ? @res : $res[0];
 }
 
 our $UNRESOLVABLE_CONDITION = \ '1 = 0';
 
 # Resolves the passed condition to a concrete query fragment and a flag
 # indicating whether this is a cross-table condition. Also an optional
-# list of non-triviail values (notmally conditions) returned as a part
+# list of non-trivial values (normally conditions) returned as a part
 # of a joinfree condition hash
-sub _resolve_condition {
-  my ($self, $cond, $as, $for, $relname) = @_;
+sub _resolve_relationship_condition {
+  my $self = shift;
 
-  my $obj_rel = !!blessed $for;
+  # self-explanatory API, modeled on the custom cond coderef:
+  # condition
+  # rel_name
+  # foreign_alias
+  # foreign_resultobj
+  # self_alias
+  # self_resultobj
+  my $args = { ref $_[0] eq 'HASH' ? %{ $_[0] } : @_ };
+
+  for ( qw( rel_name self_alias foreign_alias ) ) {
+    $self->throw_exception("Mandatory attribute '$_' is not a plain string")
+      if !defined $args->{$_} or length ref $args->{$_};
+  }
 
-  if (ref $cond eq 'CODE') {
-    my $relalias = $obj_rel ? 'me' : $as;
+  $self->throw_exception('No practical way to resolve a relationship between two objects')
+    if defined $args->{self_resultobj} and defined $args->{foreign_resultobj};
 
-    my ($crosstable_cond, $joinfree_cond) = $cond->({
-      self_alias => $obj_rel ? $as : $for,
-      foreign_alias => $relalias,
+  $args->{condition} ||= $self->relationship_info($args->{rel_name})->{cond};
+
+  if (ref $args->{condition} eq 'CODE') {
+
+    my $cref_args = {
+      rel_name => $args->{rel_name},
       self_resultsource => $self,
-      foreign_relname => $relname || ($obj_rel ? $as : $for),
-      self_rowobj => $obj_rel ? $for : undef
-    });
+      self_alias => $args->{self_alias},
+      foreign_alias => $args->{foreign_alias},
+      self_resultobj => $args->{self_resultobj},
+      foreign_resultobj => $args->{foreign_resultobj},
+    };
+
+    # legacy - never remove these!!!
+    $cref_args->{foreign_relname} = $cref_args->{rel_name};
+    $cref_args->{self_rowobj} = $cref_args->{self_resultobj};
+
+    my ($crosstable_cond, $joinfree_cond, @extra) = $args->{condition}->($cref_args);
+
+    # FIXME sanity check
+    carp_unique('A custom condition coderef can return at most 2 conditions: extra return values discarded')
+      if @extra;
 
-    my $cond_cols;
+    my @nonvalue_cols;
     if ($joinfree_cond) {
 
+      my ($joinfree_alias, $joinfree_source);
+      if (defined $args->{self_resultobj}) {
+        $joinfree_alias = $args->{foreign_alias};
+        $joinfree_source = $self->related_source($args->{rel_name});
+      }
+      elsif (defined $args->{foreign_resultobj}) {
+        $joinfree_alias = $args->{self_alias};
+        $joinfree_source = $self;
+      }
+
       # FIXME sanity check until things stabilize, remove at some point
       $self->throw_exception (
-        "A join-free condition returned for relationship '$relname' without a row-object to chain from"
-      ) unless $obj_rel;
+        "A join-free condition returned for relationship '$args->{rel_name}' without a result object to chain from"
+      ) unless $joinfree_alias;
+
+      my $fq_col_list = { map { ( "$joinfree_alias.$_" => 1 ) } $joinfree_source->columns };
 
       # FIXME another sanity check
       if (
         ref $joinfree_cond ne 'HASH'
           or
-        first { $_ !~ /^\Q$relalias.\E.+/ } keys %$joinfree_cond
+        grep { ! $fq_col_list->{$_} } keys %$joinfree_cond
       ) {
         $self->throw_exception (
-          "The join-free condition returned for relationship '$relname' must be a hash "
-         .'reference with all keys being valid columns on the related result source'
+          "The join-free condition returned for relationship '$args->{rel_name}' must be a hash "
+         .'reference with all keys being fully qualified column names of the corresponding source'
         );
       }
 
-      # normalize
-      for (values %$joinfree_cond) {
-        $_ = $_->{'='} if (
-          ref $_ eq 'HASH'
-            and
-          keys %$_ == 1
-            and
-          exists $_->{'='}
-        );
-      }
+      # see which parts of the joinfree cond are *NOT* foreign-source-column equalities
+      my $joinfree_cond_equality_columns = { map
+        {( $_ => 1 )}
+        @{ $self->schema->storage->_extract_fixed_condition_columns($joinfree_cond) }
+      };
+      @nonvalue_cols = map
+        { $_ =~ /^\Q$joinfree_alias.\E(.+)/ }
+        grep
+          { ! $joinfree_cond_equality_columns->{$_} }
+          keys %$joinfree_cond;
 
-      # see which parts of the joinfree cond are conditionals
-      my $relcol_list = { map { $_ => 1 } $self->related_source($relname)->columns };
+      return ($joinfree_cond, 0, (@nonvalue_cols ? \@nonvalue_cols : undef));
+    }
+    else {
+      return ($crosstable_cond, 1);
+    }
+  }
+  elsif (ref $args->{condition} eq 'HASH') {
 
-      for my $c (keys %$joinfree_cond) {
-        my ($colname) = $c =~ /^ (?: \Q$relalias.\E )? (.+)/x;
+    # the condition is static - use parallel arrays
+    # for a "pivot" depending on which side of the
+    # rel did we get as an object
+    my (@f_cols, @l_cols);
+    for my $fc (keys %{$args->{condition}}) {
+      my $lc = $args->{condition}{$fc};
 
-        unless ($relcol_list->{$colname}) {
-          push @$cond_cols, $colname;
-          next;
-        }
+      # FIXME STRICTMODE should probably check these are valid columns
+      $fc =~ s/^foreign\.// ||
+        $self->throw_exception("Invalid rel cond key '$fc'");
 
-        if (
-          ref $joinfree_cond->{$c}
-            and
-          ref $joinfree_cond->{$c} ne 'SCALAR'
-            and
-          ref $joinfree_cond->{$c} ne 'REF'
-        ) {
-          push @$cond_cols, $colname;
-          next;
-        }
-      }
+      $lc =~ s/^self\.// ||
+        $self->throw_exception("Invalid rel cond val '$lc'");
 
-      return wantarray ? ($joinfree_cond, 0, $cond_cols) : $joinfree_cond;
+      push @f_cols, $fc;
+      push @l_cols, $lc;
     }
-    else {
-      return wantarray ? ($crosstable_cond, 1) : $crosstable_cond;
+
+    # plain values
+    if (! defined $args->{self_resultobj} and ! defined $args->{foreign_resultobj}) {
+      return ( { map
+        {( "$args->{foreign_alias}.$f_cols[$_]" => { -ident => "$args->{self_alias}.$l_cols[$_]" } )}
+        (0..$#f_cols)
+      }, 1 ); # is crosstable
     }
-  }
-  elsif (ref $cond eq 'HASH') {
-    my %ret;
-    foreach my $k (keys %{$cond}) {
-      my $v = $cond->{$k};
-      # XXX should probably check these are valid columns
-      $k =~ s/^foreign\.// ||
-        $self->throw_exception("Invalid rel cond key ${k}");
-      $v =~ s/^self\.// ||
-        $self->throw_exception("Invalid rel cond val ${v}");
-      if (ref $for) { # Object
-        #warn "$self $k $for $v";
-        unless ($for->has_column_loaded($v)) {
-          if ($for->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.',
-              $as,
-              $for,
-              $v,
-            );
-          }
+    else {
+
+      my $cond;
+
+      my ($obj, $obj_alias, $plain_alias, $obj_cols, $plain_cols) = defined $args->{self_resultobj}
+        ? ( @{$args}{qw( self_resultobj self_alias foreign_alias )}, \@l_cols, \@f_cols )
+        : ( @{$args}{qw( foreign_resultobj foreign_alias self_alias )}, \@f_cols, \@l_cols )
+      ;
+
+      for my $i (0..$#$obj_cols) {
+        if (defined $args->{self_resultobj} and ! $obj->has_column_loaded($obj_cols->[$i])) {
+
+          $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},
+            $obj,
+            $obj_cols->[$i],
+          ) if $obj->in_storage;
+
           return $UNRESOLVABLE_CONDITION;
         }
-        $ret{$k} = $for->get_column($v);
-        #$ret{$k} = $for->get_column($v) if $for->has_column_loaded($v);
-        #warn %ret;
-      } elsif (!defined $for) { # undef, i.e. "no object"
-        $ret{$k} = undef;
-      } elsif (ref $as eq 'HASH') { # reverse hashref
-        $ret{$v} = $as->{$k};
-      } elsif (ref $as) { # reverse object
-        $ret{$v} = $as->get_column($k);
-      } elsif (!defined $as) { # undef, i.e. "no reverse object"
-        $ret{$v} = undef;
-      } else {
-        $ret{"${as}.${k}"} = { -ident => "${for}.${v}" };
+        else {
+          $cond->{"$plain_alias.$plain_cols->[$i]"} = $obj->get_column($obj_cols->[$i]);
+        }
       }
-    }
 
-    return wantarray
-      ? ( \%ret, ($obj_rel || !defined $as || ref $as) ? 0 : 1 )
-      : \%ret
-    ;
+      return ($cond, 0); # joinfree
+    }
   }
-  elsif (ref $cond eq 'ARRAY') {
-    my (@ret, $crosstable);
-    for (@$cond) {
-      my ($cond, $crosstab) = $self->_resolve_condition($_, $as, $for, $relname);
-      push @ret, $cond;
-      $crosstable ||= $crosstab;
+  elsif (ref $args->{condition} eq 'ARRAY') {
+    if (@{$args->{condition}} == 0) {
+      return $UNRESOLVABLE_CONDITION;
+    }
+    elsif (@{$args->{condition}} == 1) {
+      return $self->_resolve_relationship_condition({
+        %$args,
+        condition => $args->{condition}[0],
+      });
+    }
+    else {
+      # FIXME - we are discarding nonvalues here... likely incorrect...
+      # then again - the entire thing is an OR, so we *can't* use
+      # the values anyway
+      # Return a hard crosstable => 1 to ensure nothing tries to use
+      # the result in such manner
+      my @ret;
+      for (@{$args->{condition}}) {
+        my ($cond) = $self->_resolve_relationship_condition({
+          %$args,
+          condition => $_,
+        });
+        push @ret, $cond;
+      }
+      return (\@ret, 1); # forced cross-tab
     }
-    return wantarray ? (\@ret, $crosstable) : \@ret;
   }
   else {
-    $self->throw_exception ("Can't handle condition $cond for relationship '$relname' yet :(");
+    $self->throw_exception ("Can't handle condition $args->{condition} for relationship '$args->{rel_name}' yet :(");
   }
+
+  die "not supposed to get here - missing return()";
 }
 
 =head2 related_source
 
 =over 4
 
-=item Arguments: $relname
+=item Arguments: $rel_name
 
-=item Return value: $source
+=item Return Value: $source
 
 =back
 
@@ -1784,9 +1974,9 @@ sub related_source {
 
 =over 4
 
-=item Arguments: $relname
+=item Arguments: $rel_name
 
-=item Return value: $classname
+=item Return Value: $classname
 
 =back
 
@@ -1806,9 +1996,9 @@ sub related_class {
 
 =over 4
 
-=item Arguments: None
+=item Arguments: none
 
-=item Return value: $source_handle
+=item Return Value: L<$source_handle|DBIx::Class::ResultSourceHandle>
 
 =back
 
@@ -1925,7 +2115,7 @@ Creates a new ResultSource object.  Not normally called directly by end users.
 
 =item Arguments: 1/0 (default: 0)
 
-=item Return value: 1/0
+=item Return Value: 1/0
 
 =back
 
@@ -1936,9 +2126,9 @@ metadata from storage as necessary.  This is *deprecated*, and
 should not be used.  It will be removed before 1.0.
 
 
-=head1 AUTHORS
+=head1 AUTHOR AND CONTRIBUTORS
 
-Matt S. Trout <mst@shadowcatsystems.co.uk>
+See L<AUTHOR|DBIx::Class/AUTHOR> and L<CONTRIBUTORS|DBIx::Class/CONTRIBUTORS> in DBIx::Class
 
 =head1 LICENSE