From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 10 Jun 2014 11:54:32 +0000 (+0200)
Subject: Fix incorrect handling of custom relationship conditions containing literals
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=f8193780f1e03bc6f1316204a03bf604faf9267b;p=dbsrgits%2FDBIx-Class-Historic.git

Fix incorrect handling of custom relationship conditions containing literals
---

diff --git a/Changes b/Changes
index c82b355..1ae385c 100644
--- a/Changes
+++ b/Changes
@@ -12,6 +12,8 @@ Revision history for DBIx::Class
         - Fix on_connect_* not always firing in some cases - a race condition
           existed between storage accessor setters and the determine_driver
           routines, triggering a connection before the set-cycle is finished
+        - Fix incorrect handling of custom relationship conditions returning
+          SQLA literal expressions
         - Fix multi-value literal populate not working with simplified bind
           specifications
         - Massively improve the implied resultset condition parsing - now all
diff --git a/lib/DBIx/Class/Relationship/Base.pm b/lib/DBIx/Class/Relationship/Base.pm
index a41d6b4..16d213d 100644
--- a/lib/DBIx/Class/Relationship/Base.pm
+++ b/lib/DBIx/Class/Relationship/Base.pm
@@ -483,11 +483,8 @@ sub related_resultset {
       $rsrc->_resolve_condition( $rel_info->{cond}, $rel, $self, $rel )
     }
     catch {
-      if ($self->in_storage) {
-        $self->throw_exception ($_);
-      }
-
-      $DBIx::Class::ResultSource::UNRESOLVABLE_CONDITION;  # RV
+      $self->throw_exception ($_) if $self->in_storage;
+      $DBIx::Class::ResultSource::UNRESOLVABLE_CONDITION;  # RV, no return()
     };
 
     # keep in mind that the following if() block is part of a do{} - no return()s!!!
@@ -642,14 +639,18 @@ sub new_related {
     my $rsrc = $self->result_source;
     my $rel_info = $rsrc->relationship_info($rel)
       or $self->throw_exception( "No such relationship '$rel'" );
-    my (undef, $crosstable, $cond_targets) = $rsrc->_resolve_condition (
+    my (undef, $crosstable, $nonequality_foreign_columns) = $rsrc->_resolve_condition (
       $rel_info->{cond}, $rel, $self, $rel
     );
 
     $self->throw_exception("Custom relationship '$rel' does not resolve to a join-free condition fragment")
       if $crosstable;
 
-    if (my @unspecified_rel_condition_chunks = grep { ! exists $values->{$_} } @{$cond_targets||[]} ) {
+    if (
+      $nonequality_foreign_columns
+        and
+      my @unspecified_rel_condition_chunks = grep { ! exists $values->{$_} } @$nonequality_foreign_columns
+    ) {
       $self->throw_exception(sprintf (
         "Custom relationship '%s' not definitive - returns conditions instead of values for column(s): %s",
         $rel,
@@ -818,16 +819,17 @@ sub set_from_related {
   #
   # sanity check - currently throw when a complex coderef rel is encountered
   # FIXME - should THROW MOAR!
-  my ($cond, $crosstable, $cond_targets) = $rsrc->_resolve_condition (
+  my ($cond, $crosstable, $nonequality_foreign_columns) = $rsrc->_resolve_condition (
     $rel_info->{cond}, $f_obj, $rel, $rel
   );
   $self->throw_exception("Custom relationship '$rel' does not resolve to a join-free condition fragment")
     if $crosstable;
+
   $self->throw_exception(sprintf (
     "Custom relationship '%s' not definitive - returns conditions instead of values for column(s): %s",
     $rel,
-    map { "'$_'" } @$cond_targets
-  )) if $cond_targets;
+    map { "'$_'" } @$nonequality_foreign_columns
+  )) if $nonequality_foreign_columns;
 
   $self->set_columns($cond);
 
diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index e1ebbf7..00d257f 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -1697,7 +1697,7 @@ sub _resolve_condition {
       self_rowobj => $obj_rel ? $for : undef
     });
 
-    my $cond_cols;
+    my @nonvalue_cols;
     if ($joinfree_cond) {
 
       # FIXME sanity check until things stabilize, remove at some point
@@ -1705,51 +1705,34 @@ sub _resolve_condition {
         "A join-free condition returned for relationship '$rel_name' without a row-object to chain from"
       ) unless $obj_rel;
 
+      my $foreign_src_col_list = { map { ( "$relalias.$_" => 1 ) } $self->related_source($rel_name)->columns };
       # FIXME another sanity check
       if (
         ref $joinfree_cond ne 'HASH'
           or
-        first { $_ !~ /^\Q$relalias.\E.+/ } keys %$joinfree_cond
+        grep { ! $foreign_src_col_list } keys %$joinfree_cond
       ) {
         $self->throw_exception (
           "The join-free condition returned for relationship '$rel_name' must be a hash "
-         .'reference with all keys being valid columns on the related result source'
+         .'reference with all keys being fully qualified column names of the foreign source'
         );
       }
 
-      # normalize
-      for (values %$joinfree_cond) {
-        $_ = $_->{'='} if (
-          ref $_ eq 'HASH'
-            and
-          keys %$_ == 1
-            and
-          exists $_->{'='}
-        );
-      }
-
-      # see which parts of the joinfree cond are conditionals
-      my $relcol_list = { map { $_ => 1 } $self->related_source($rel_name)->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
-          ! is_literal_value( $joinfree_cond->{$c} )
-        ) {
-          push @$cond_cols, $colname;
-          next;
-        }
-      }
-
-      return wantarray ? ($joinfree_cond, 0, $cond_cols) : $joinfree_cond;
+      # 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$relalias.\E(.+)/ }
+        grep
+          { ! $joinfree_cond_equality_columns->{$_} }
+          keys %$joinfree_cond;
+
+      return wantarray
+        ? ($joinfree_cond, 0, (@nonvalue_cols ? \@nonvalue_cols : undef))
+        : $joinfree_cond
+      ;
     }
     else {
       return wantarray ? ($crosstable_cond, 1) : $crosstable_cond;
diff --git a/lib/DBIx/Class/Row.pm b/lib/DBIx/Class/Row.pm
index 9127c94..cb902d2 100644
--- a/lib/DBIx/Class/Row.pm
+++ b/lib/DBIx/Class/Row.pm
@@ -200,7 +200,7 @@ sub new {
     my ($related,$inflated);
 
     foreach my $key (keys %$attrs) {
-      if (ref $attrs->{$key}) {
+      if (ref $attrs->{$key} and ! is_literal_value($attrs->{$key}) ) {
         ## Can we extract this lot to use with update(_or .. ) ?
         $new->throw_exception("Can't do multi-create without result source")
           unless $rsrc;
diff --git a/t/lib/DBICTest/Schema/Artist.pm b/t/lib/DBICTest/Schema/Artist.pm
index a99eb7e..2bc7c4b 100644
--- a/t/lib/DBICTest/Schema/Artist.pm
+++ b/t/lib/DBICTest/Schema/Artist.pm
@@ -66,11 +66,11 @@ __PACKAGE__->has_many(
       if @missing_args;
 
     return (
-      { "$args->{foreign_alias}.artist" => { '=' => { -ident => "$args->{self_alias}.artistid"} },
+      { "$args->{foreign_alias}.artist" => { '=' => \ "$args->{self_alias}.artistid" },
         "$args->{foreign_alias}.year"   => { '>' => 1979, '<' => 1990 },
       },
       $args->{self_rowobj} && {
-        "$args->{foreign_alias}.artist" => $args->{self_rowobj}->artistid,
+        "$args->{foreign_alias}.artist" => { '=' => \[ '?',  $args->{self_rowobj}->artistid ] },
         "$args->{foreign_alias}.year"   => { '>' => 1979, '<' => 1990 },
       }
     );
diff --git a/t/relationship/custom.t b/t/relationship/custom.t
index 3d47fb1..61f709c 100644
--- a/t/relationship/custom.t
+++ b/t/relationship/custom.t
@@ -43,7 +43,7 @@ is_same_sql_bind(
   )',
   [
     [
-      { sqlt_datatype => 'integer', dbic_colname => 'me.artist' }
+      {}
         => 21
     ],
     [