From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 10 Aug 2016 08:12:22 +0000 (+0200)
Subject: Tighten up value inferrence in relationship resolution
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=d8516e922b021c1aa8b0626694cb472b5407573b;p=dbsrgits%2FDBIx-Class.git

Tighten up value inferrence in relationship resolution

Read under -w
---

diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index ee5704f..8b01a88 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -2495,12 +2495,12 @@ sub _resolve_relationship_condition {
         $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 inferred_values from individual 'OR' branches here
+          # see @nonvalues checks below
           $subcond->{$_} and push @{$ret->{$_}}, $subcond->{$_} for (qw(condition join_free_condition));
         }
       }
@@ -2535,31 +2535,43 @@ sub _resolve_relationship_condition {
 
     my $jfc_eqs = extract_equality_conditions( $jfc, 'consider_nulls' );
 
-    if (keys %$jfc_eqs) {
-
-      for (keys %$jfc) {
+    for (keys %$jfc) {
+      if( $_ =~ /^-/ ) {
+        push @nonvalues, { $_ => $jfc->{$_} };
+      }
+      else {
         # $jfc is fully qualified by definition
-        my ($col) = $_ =~ /\.(.+)/;
+        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->{$_};
         }
         elsif ( !$args->{infer_values_based_on} or ! exists $args->{infer_values_based_on}{$col} ) {
-          push @nonvalues, $col;
+          push @nonvalues, { $_ => $jfc->{$_} };
         }
       }
-
-      # all or nothing
-      delete $ret->{inferred_values} if @nonvalues;
     }
+
+    # all or nothing
+    delete $ret->{inferred_values} if @nonvalues;
   }
 
   # did the user explicitly ask
   if ($args->{infer_values_based_on}) {
 
     $self->throw_exception(sprintf (
-      "Unable to complete value inferrence - custom $exception_rel_id returns conditions instead of values for column(s): %s",
-      map { "'$_'" } @nonvalues
+      "Unable to complete value inferrence - $exception_rel_id results in expression(s) instead of definitive values: %s",
+      do {
+        # FIXME - used for diag only, but still icky
+        my $sqlm = $self->schema->storage->sql_maker;
+        local $sqlm->{quote_char};
+        local $sqlm->{_dequalify_idents} = 1;
+        ($sqlm->_recurse_where({ -and => \@nonvalues }))[0]
+      }
     )) if @nonvalues;
 
 
diff --git a/t/relationship/core.t b/t/relationship/core.t
index de6afd7..6ebf94f 100644
--- a/t/relationship/core.t
+++ b/t/relationship/core.t
@@ -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' );
 
diff --git a/t/relationship/custom.t b/t/relationship/custom.t
index 32c8cf8..2646505 100644
--- a/t/relationship/custom.t
+++ b/t/relationship/custom.t
@@ -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