Even more robust _resolve_condition shim - guess rel_name when needed
[dbsrgits/DBIx-Class.git] / lib / DBIx / Class / ResultSource.pm
index 9489f49..fb4c27d 100644 (file)
@@ -3,17 +3,19 @@ package DBIx::Class::ResultSource;
 use strict;
 use warnings;
 
+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';
 use Scalar::Util qw/blessed weaken isweak/;
-use namespace::clean;
 
-use base qw/DBIx::Class/;
+use namespace::clean;
 
 __PACKAGE__->mk_group_accessors(simple => qw/
   source_name name source_info
@@ -83,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:
 
@@ -93,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:
 
@@ -132,7 +134,7 @@ sub new {
 
 =item Arguments: @columns
 
-=item Return value: The ResultSource object
+=item Return Value: L<$result_source|/new>
 
 =back
 
@@ -145,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
@@ -200,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>.
 
@@ -298,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
 
@@ -342,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
 
@@ -363,7 +365,7 @@ sub has_column {
 
 =item Arguments: $colname
 
-=item Return value: Hashref of info
+=item Return Value: Hashref of info
 
 =back
 
@@ -411,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
 
@@ -437,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
 
@@ -491,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...?',
         ));
       }
     }
@@ -511,7 +513,7 @@ sub columns_info {
 
 =item Arguments: @colnames
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -529,7 +531,7 @@ broken result source.
 
 =item Arguments: $colname
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -567,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
 
@@ -587,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);
@@ -601,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
 
@@ -619,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(
@@ -630,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
@@ -640,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
 
@@ -663,7 +686,7 @@ sub sequence {
 
 =item Arguments: $name?, \@colnames
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -729,7 +752,7 @@ sub add_unique_constraint {
 
 =item Arguments: @constraints
 
-=item Return value: undefined
+=item Return Value: not defined
 
 =back
 
@@ -781,7 +804,7 @@ sub add_unique_constraints {
 
 =item Arguments: \@colnames
 
-=item Return value: Constraint name
+=item Return Value: Constraint name
 
 =back
 
@@ -815,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
 
@@ -839,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
 
@@ -865,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
 
@@ -893,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
 
@@ -960,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
 
@@ -983,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
 
@@ -1007,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
 
@@ -1020,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
 
@@ -1041,6 +1117,20 @@ sub resultset {
   );
 }
 
+=head2 name
+
+=over 4
+
+=item Arguments: none
+
+=item Result value: $name
+
+=back
+
+Returns the name of the result source, which will typically be the table
+name. This may be a scalar reference if the result source has a non-standard
+name.
+
 =head2 source_name
 
 =over 4
@@ -1067,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
 
@@ -1087,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
 
@@ -1123,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
 
@@ -1143,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
@@ -1269,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.
 
@@ -1289,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
 
@@ -1320,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
 
@@ -1360,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
@@ -1380,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);
@@ -1434,6 +1520,32 @@ sub _compare_relationship_keys {
   ;
 }
 
+# optionally takes either an arrayref of column names, or a hashref of already
+# retrieved colinfos
+# returns an arrayref of column names of the shortest unique constraint
+# (matching some of the input if any), giving preference to the PK
+sub _identifying_column_set {
+  my ($self, $cols) = @_;
+
+  my %unique = $self->unique_constraints;
+  my $colinfos = ref $cols eq 'HASH' ? $cols : $self->columns_info($cols||());
+
+  # always prefer the PK first, and then shortest constraints first
+  USET:
+  for my $set (delete $unique{primary}, sort { @$a <=> @$b } (values %unique) ) {
+    next unless $set && @$set;
+
+    for (@$set) {
+      next USET unless ($colinfos->{$_} && !$colinfos->{$_}{is_nullable} );
+    }
+
+    # copy so we can mangle it at will
+    return [ @$set ];
+  }
+
+  return undef;
+}
+
 # Returns the {from} structure used to express JOIN conditions
 sub _resolve_join {
   my ($self, $join, $alias, $seen, $jpath, $parent_force_left) = @_;
@@ -1447,7 +1559,7 @@ sub _resolve_join {
 
   $jpath = [@$jpath]; # copy
 
-  if (not defined $join) {
+  if (not defined $join or not length $join) {
     return ();
   }
   elsif (ref $join eq 'ARRAY') {
@@ -1503,12 +1615,12 @@ sub _resolve_join {
                 ,
                -join_path => [@$jpath, { $join => $as } ],
                -is_single => (
-                  $rel_info->{attrs}{accessor}
-                    &&
+                  (! $rel_info->{attrs}{accessor})
+                    or
                   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)
           ];
@@ -1525,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}
@@ -1542,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}) {
@@ -1557,269 +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;
+
+  # 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->{$_};
+  }
+
+  $self->throw_exception('No practical way to resolve a relationship between two objects')
+    if defined $args->{self_resultobj} and defined $args->{foreign_resultobj};
 
-  my $obj_rel = !!blessed $for;
+  $args->{condition} ||= $self->relationship_info($args->{rel_name})->{cond};
 
-  if (ref $cond eq 'CODE') {
-    my $relalias = $obj_rel ? 'me' : $as;
+  if (ref $args->{condition} eq 'CODE') {
 
-    my ($crosstable_cond, $joinfree_cond) = $cond->({
-      self_alias => $obj_rel ? $as : $for,
-      foreign_alias => $relalias,
+    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);
 
-    my $cond_cols;
+    # FIXME sanity check
+    carp_unique('A custom condition coderef can return at most 2 conditions: extra return values discarded')
+      if @extra;
+
+    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'
-        );
-      }
-
-      # normalize
-      for (values %$joinfree_cond) {
-        $_ = $_->{'='} if (
-          ref $_ eq 'HASH'
-            and
-          keys %$_ == 1
-            and
-          exists $_->{'='}
+          "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'
         );
       }
 
-      # see which parts of the joinfree cond are conditionals
-      my $relcol_list = { map { $_ => 1 } $self->related_source($relname)->columns };
-
-      for my $c (keys %$joinfree_cond) {
-        my ($colname) = $c =~ /^ (?: \Q$relalias.\E )? (.+)/x;
-
-        unless ($relcol_list->{$colname}) {
-          push @$cond_cols, $colname;
-          next;
-        }
-
-        if (
-          ref $joinfree_cond->{$c}
-            and
-          ref $joinfree_cond->{$c} ne 'SCALAR'
-            and
-          ref $joinfree_cond->{$c} ne 'REF'
-        ) {
-          push @$cond_cols, $colname;
-          next;
-        }
-      }
+      # 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;
 
-      return wantarray ? ($joinfree_cond, 0, $cond_cols) : $joinfree_cond;
+      return ($joinfree_cond, 0, (@nonvalue_cols ? \@nonvalue_cols : undef));
     }
     else {
-      return wantarray ? ($crosstable_cond, 1) : $crosstable_cond;
+      return ($crosstable_cond, 1);
     }
   }
-  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,
-            );
-          }
-          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}" };
-      }
+  elsif (ref $args->{condition} eq 'HASH') {
+
+    # 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};
+
+      # FIXME STRICTMODE should probably check these are valid columns
+      $fc =~ s/^foreign\.// ||
+        $self->throw_exception("Invalid rel cond key '$fc'");
+
+      $lc =~ s/^self\.// ||
+        $self->throw_exception("Invalid rel cond val '$lc'");
+
+      push @f_cols, $fc;
+      push @l_cols, $lc;
     }
 
-    return wantarray
-      ? ( \%ret, ($obj_rel || !defined $as || ref $as) ? 0 : 1 )
-      : \%ret
-    ;
-  }
-  elsif (ref $cond eq 'ARRAY') {
-    my (@ret, $crosstable);
-    for (@$cond) {
-      my ($cond, $crosstab) = $self->_resolve_condition($_, $as, $for, $relname);
-      push @ret, $cond;
-      $crosstable ||= $crosstab;
+    # 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
     }
-    return wantarray ? (\@ret, $crosstable) : \@ret;
-  }
-  else {
-    $self->throw_exception ("Can't handle condition $cond for relationship '$relname' yet :(");
-  }
-}
+    else {
 
-# Accepts one or more relationships for the current source and returns an
-# array of column names for each of those relationships. Column names are
-# prefixed relative to the current source, in accordance with where they appear
-# in the supplied relationships.
+      my $cond;
 
-sub _resolve_prefetch {
-  my ($self, $pre, $alias, $alias_map, $order, $collapse, $pref_path) = @_;
-  $pref_path ||= [];
+      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 )
+      ;
 
-  if (not defined $pre) {
-    return ();
-  }
-  elsif( ref $pre eq 'ARRAY' ) {
-    return
-      map { $self->_resolve_prefetch( $_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ) }
-        @$pre;
-  }
-  elsif( ref $pre eq 'HASH' ) {
-    my @ret =
-    map {
-      $self->_resolve_prefetch($_, $alias, $alias_map, $order, $collapse, [ @$pref_path ] ),
-      $self->related_source($_)->_resolve_prefetch(
-               $pre->{$_}, "${alias}.$_", $alias_map, $order, $collapse, [ @$pref_path, $_] )
-    } keys %$pre;
-    return @ret;
-  }
-  elsif( ref $pre ) {
-    $self->throw_exception(
-      "don't know how to resolve prefetch reftype ".ref($pre));
-  }
-  else {
-    my $p = $alias_map;
-    $p = $p->{$_} for (@$pref_path, $pre);
+      for my $i (0..$#$obj_cols) {
+        if (defined $args->{self_resultobj} and ! $obj->has_column_loaded($obj_cols->[$i])) {
 
-    $self->throw_exception (
-      "Unable to resolve prefetch '$pre' - join alias map does not contain an entry for path: "
-      . join (' -> ', @$pref_path, $pre)
-    ) if (ref $p->{-join_aliases} ne 'ARRAY' or not @{$p->{-join_aliases}} );
-
-    my $as = shift @{$p->{-join_aliases}};
-
-    my $rel_info = $self->relationship_info( $pre );
-    $self->throw_exception( $self->source_name . " has no such relationship '$pre'" )
-      unless $rel_info;
-    my $as_prefix = ($alias =~ /^.*?\.(.+)$/ ? $1.'.' : '');
-    my $rel_source = $self->related_source($pre);
-
-    if ($rel_info->{attrs}{accessor} && $rel_info->{attrs}{accessor} eq 'multi') {
-      $self->throw_exception(
-        "Can't prefetch has_many ${pre} (join cond too complex)")
-        unless ref($rel_info->{cond}) eq 'HASH';
-      my $dots = @{[$as_prefix =~ m/\./g]} + 1; # +1 to match the ".${as_prefix}"
-
-      if (my ($fail) = grep { @{[$_ =~ m/\./g]} == $dots }
-                         keys %{$collapse}) {
-        my ($last) = ($fail =~ /([^\.]+)$/);
-        carp (
-          "Prefetching multiple has_many rels ${last} and ${pre} "
-          .(length($as_prefix)
-            ? "at the same level (${as_prefix}) "
-            : "at top level "
-          )
-          . 'will explode the number of row objects retrievable via ->next or ->all. '
-          . 'Use at your own risk.'
-        );
-      }
+          $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;
 
-      #my @col = map { (/^self\.(.+)$/ ? ("${as_prefix}.$1") : ()); }
-      #              values %{$rel_info->{cond}};
-      $collapse->{".${as_prefix}${pre}"} = [ $rel_source->_pri_cols ];
-        # action at a distance. prepending the '.' allows simpler code
-        # in ResultSet->_collapse_result
-      my @key = map { (/^foreign\.(.+)$/ ? ($1) : ()); }
-                    keys %{$rel_info->{cond}};
-      push @$order, map { "${as}.$_" } @key;
-
-      if (my $rel_order = $rel_info->{attrs}{order_by}) {
-        # this is kludgy and incomplete, I am well aware
-        # but the parent method is going away entirely anyway
-        # so sod it
-        my $sql_maker = $self->storage->sql_maker;
-        my ($orig_ql, $orig_qr) = $sql_maker->_quote_chars;
-        my $sep = $sql_maker->name_sep;
-
-        # install our own quoter, so we can catch unqualified stuff
-        local $sql_maker->{quote_char} = ["\x00", "\xFF"];
-
-        my $quoted_prefix = "\x00${as}\xFF";
-
-        for my $chunk ( $sql_maker->_order_by_chunks ($rel_order) ) {
-          my @bind;
-          ($chunk, @bind) = @$chunk if ref $chunk;
-
-          $chunk = "${quoted_prefix}${sep}${chunk}"
-            unless $chunk =~ /\Q$sep/;
-
-          $chunk =~ s/\x00/$orig_ql/g;
-          $chunk =~ s/\xFF/$orig_qr/g;
-          push @$order, \[$chunk, @bind];
+          return $UNRESOLVABLE_CONDITION;
+        }
+        else {
+          $cond->{"$plain_alias.$plain_cols->[$i]"} = $obj->get_column($obj_cols->[$i]);
         }
       }
-    }
 
-    return map { [ "${as}.$_", "${as_prefix}${pre}.$_", ] }
-      $rel_source->columns;
+      return ($cond, 0); # joinfree
+    }
   }
+  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
+    }
+  }
+  else {
+    $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
 
@@ -1850,9 +1974,9 @@ sub related_source {
 
 =over 4
 
-=item Arguments: $relname
+=item Arguments: $rel_name
 
-=item Return value: $classname
+=item Return Value: $classname
 
 =back
 
@@ -1872,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
 
@@ -1897,16 +2021,9 @@ sub handle {
   });
 }
 
-{
-  my $global_phase_destroy;
-
-  # SpeedyCGI runs END blocks every cycle but keeps object instances
-  # hence we have to disable the globaldestroy hatch, and rely on the
-  # eval trap below (which appears to work, but is risky done so late)
-  END { $global_phase_destroy = 1 unless $CGI::SpeedyCGI::i_am_speedy }
-
-  sub DESTROY {
-    return if $global_phase_destroy;
+my $global_phase_destroy;
+sub DESTROY {
+  return if $global_phase_destroy ||= in_global_destruction;
 
 ######
 # !!! ACHTUNG !!!!
@@ -1918,25 +2035,21 @@ sub handle {
 # we are trying to save to reattach back to the source we are destroying.
 # The relevant code checking refcounts is in ::Schema::DESTROY()
 
-    # if we are not a schema instance holder - we don't matter
-    return if(
-      ! ref $_[0]->{schema}
-        or
-      isweak $_[0]->{schema}
-    );
-
-    # weaken our schema hold forcing the schema to find somewhere else to live
-    # during global destruction (if we have not yet bailed out) this will throw
-    # which will serve as a signal to not try doing anything else
-    local $@;
-    eval {
-      weaken $_[0]->{schema};
-      1;
-    } or do {
-      $global_phase_destroy = 1;
-      return;
-    };
+  # if we are not a schema instance holder - we don't matter
+  return if(
+    ! ref $_[0]->{schema}
+      or
+    isweak $_[0]->{schema}
+  );
 
+  # weaken our schema hold forcing the schema to find somewhere else to live
+  # during global destruction (if we have not yet bailed out) this will throw
+  # which will serve as a signal to not try doing anything else
+  # however beware - on older perls the exception seems randomly untrappable
+  # due to some weird race condition during thread joining :(((
+  local $@;
+  eval {
+    weaken $_[0]->{schema};
 
     # if schema is still there reintroduce ourselves with strong refs back to us
     if ($_[0]->{schema}) {
@@ -1946,7 +2059,13 @@ sub handle {
         $srcregs->{$_} = $_[0] if $srcregs->{$_} == $_[0];
       }
     }
-  }
+
+    1;
+  } or do {
+    $global_phase_destroy = 1;
+  };
+
+  return;
 }
 
 sub STORABLE_freeze { Storable::nfreeze($_[0]->handle) }
@@ -1996,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
 
@@ -2007,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