From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 9 Sep 2014 23:38:43 +0000 (+0200)
Subject: Fix incorrect cond construction in _minimal_valueset_satisfying_constraint
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=76cc4546dfc51e7d83add9a90af1eb2d5a7d156c;p=dbsrgits%2FDBIx-Class-Historic.git

Fix incorrect cond construction in _minimal_valueset_satisfying_constraint

The function in question introduced in d681f1bb (which builds upon work in
8e40a627) correctly uses _extract_fixed_condition_columns, but then fails to
account for all literals having their leading { '=' => ... } being stripped,
nor does it consider UNRESOLVABLE_CONDITION as a valid return value.

Tests and fixes to get this rolling, thanks go to Lianna Eeftinck for testing
and reporting \o/
---

diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index 4550af8..fde2ac5 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -1567,18 +1567,21 @@ sub _minimal_valueset_satisfying_constraint {
 
   my $cols;
   for my $col ($self->unique_constraint_columns($args->{constraint_name}) ) {
-    if( ! exists $vals->{$col} ) {
-      $cols->{missing}{$col} = 1;
+    if( ! exists $vals->{$col} or ( $vals->{$col}||'' ) eq UNRESOLVABLE_CONDITION ) {
+      $cols->{missing}{$col} = undef;
     }
     elsif( ! defined $vals->{$col} ) {
-      $cols->{$args->{carp_on_nulls} ? 'undefined' : 'missing'}{$col} = 1;
+      $cols->{$args->{carp_on_nulls} ? 'undefined' : 'missing'}{$col} = undef;
     }
     else {
-      $cols->{present}{$col} = 1;
+      # we need to inject back the '=' as _extract_fixed_condition_columns
+      # will strip it from literals and values alike, resulting in an invalid
+      # condition in the end
+      $cols->{present}{$col} = { '=' => $vals->{$col} };
     }
 
     $cols->{fc}{$col} = 1 if (
-      ! ( $cols->{missing} || {})->{$col}
+      ( ! $cols->{missing} or ! exists $cols->{missing}{$col} )
         and
       keys %{ $args->{columns_info}{$col}{_filter_info} || {} }
     );
@@ -1609,10 +1612,7 @@ sub _minimal_valueset_satisfying_constraint {
     ));
   }
 
-  return { map
-    { $_ => $vals->{$_} }
-    ( keys %{$cols->{present}}, keys %{$cols->{undefined}} )
-  };
+  return { map { %{ $cols->{$_}||{} } } qw(present undefined) };
 }
 
 # Returns the {from} structure used to express JOIN conditions
diff --git a/t/60core.t b/t/60core.t
index 62299c3..f92159b 100644
--- a/t/60core.t
+++ b/t/60core.t
@@ -130,6 +130,13 @@ throws_ok {
 
 is($schema->resultset("Artist")->count, 4, 'count ok');
 
+# test find on an unresolvable condition
+is(
+  $schema->resultset('Artist')->find({ artistid => [ -and => 1, 2 ]}),
+  undef
+);
+
+
 # test find_or_new
 {
   my $existing_obj = $schema->resultset('Artist')->find_or_new({
diff --git a/t/resultset/find_on_subquery_cond.t b/t/resultset/find_on_subquery_cond.t
new file mode 100644
index 0000000..af2ca51
--- /dev/null
+++ b/t/resultset/find_on_subquery_cond.t
@@ -0,0 +1,34 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+
+use lib qw(t/lib);
+use DBICTest;
+
+my $schema = DBICTest->init_schema();
+my $rs = $schema->resultset('Artist');
+
+for my $id (
+  2,
+  \' = 2 ',
+  \[ '= ?', 2 ],
+) {
+  lives_ok {
+    is( $rs->find({ artistid => $id })->id, 2 )
+  } "Correctly found artist with id of @{[ explain $id ]}";
+}
+
+for my $id (
+  2,
+  \'2',
+  \[ '?', 2 ],
+) {
+  my $cond = { artistid => { '=', $id } };
+  lives_ok {
+    is( $rs->find($cond)->id, 2 )
+  } "Correctly found artist with id of @{[ explain $cond ]}";
+}
+
+done_testing;