From: Peter Rabbitson <ribasushi@cpan.org>
Date: Sun, 4 Apr 2010 00:17:37 +0000 (+0000)
Subject: Fix RTs #56062 and #56258
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=953d164e8af3620382535213faf758682b391a14;p=scpubgit%2FQ-Branch.git

Fix RTs #56062 and #56258
---

diff --git a/Changes b/Changes
index e5f7447..724962f 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 Revision history for SQL::Abstract
 
+    - Fix multiple generic op handling regressions by
+      reverting the auto-equality assumption (turned out
+      to be a very very bad idea)
+
 revision 1.63  2010-03-24 09:56 (UTC)
 ----------------------------
     - Add ILIKE to the core list of comparision ops
diff --git a/lib/SQL/Abstract.pm b/lib/SQL/Abstract.pm
index 32734a3..7eb2273 100644
--- a/lib/SQL/Abstract.pm
+++ b/lib/SQL/Abstract.pm
@@ -15,7 +15,7 @@ use Scalar::Util ();
 # GLOBALS
 #======================================================================
 
-our $VERSION  = '1.63';
+our $VERSION  = '1.63_01';
 
 # This would confuse some packagers
 #$VERSION      = eval $VERSION; # numify for warning-free dev releases
@@ -82,14 +82,6 @@ sub new {
   # default comparison is "=", but can be overridden
   $opt{cmp} ||= '=';
 
-  # generic SQL comparison operators
-  my $anchored_cmp_ops = join ('|', map { '^' . $_ . '$' } (
-    '(?:is \s+)? (?:not \s+)? i? like',
-    'is',
-    (map { quotemeta($_) } (qw/ < > != <> = <= >= /) ),
-  ));
-  $opt{cmp_ops} = qr/$anchored_cmp_ops/ix;
-
   # try to recognize which are the 'equality' and 'unequality' ops
   # (temporary quickfix, should go through a more seasoned API)
   $opt{equality_op}   = qr/^(\Q$opt{cmp}\E|is|(is\s+)?like)$/i;
@@ -382,7 +374,6 @@ sub _recurse_where {
   # dispatch on appropriate method according to refkind of $where
   my $method = $self->_METHOD_FOR_refkind("_where", $where);
 
-
   my ($sql, @bind) =  $self->$method($where, $logic); 
 
   # DBIx::Class directly calls _recurse_where in scalar context, so 
@@ -492,7 +483,9 @@ sub _where_HASHREF {
         }
         else {
           $self->debug("Generic unary OP: $k - recursing as function");
-          $self->_where_func_generic ($op, $v);
+          my ($sql, @bind) = $self->_where_func_generic ($op, $v);
+          $sql = "($sql)" unless $self->{_nested_func_lhs} eq $k;  # top level vs nested
+          ($sql, @bind);
         }
       }
       else {
@@ -526,9 +519,9 @@ sub _where_func_generic {
     },
   });
 
-  $sql = sprintf ('%s%s',
+  $sql = sprintf ('%s %s',
     $self->_sqlcase($op),
-    ($op =~ $self->{cmp_ops}) ? " $sql" : "( $sql )",
+    $sql,
   );
 
   return ($sql, @bind);
@@ -714,16 +707,15 @@ sub _where_hashpair_HASHREF {
 
         FALLBACK => sub {       # CASE: col => {op/func => $stuff}
 
-          # if we are starting to nest and the first func is not a cmp op
-          # assume equality
-          my $prefix;
-          unless ($self->{_nested_func_lhs}) {
-            $self->{_nested_func_lhs} = $k;
-            $prefix = $self->{cmp} unless $op =~ $self->{cmp_ops};
-          }
+          # retain for proper column type bind
+          $self->{_nested_func_lhs} ||= $k;
 
           ($sql, @bind) = $self->_where_func_generic ($op, $val);
-          $sql = join ' ', $self->_convert($self->_quote($k)), $prefix||(), $sql;
+
+          $sql = join (' ',
+            $self->_convert($self->_quote($k)),
+            $self->{_nested_func_lhs} eq $k ? $sql : "($sql)",  # top level vs nested
+          );
         },
       });
     }
diff --git a/t/01generate.t b/t/01generate.t
index 8033c97..ee56e60 100644
--- a/t/01generate.t
+++ b/t/01generate.t
@@ -587,9 +587,9 @@ my @tests = (
       {
               func   => 'select',
               new    => {bindtype => 'columns'},
-              args   => ['test', '*', [ Y => { -max => { -LENGTH => { -min => 'x' } } } ] ],
-              stmt   => 'SELECT * FROM test WHERE ( Y = MAX( LENGTH( MIN( ? ) ) ) )',
-              stmt_q => 'SELECT * FROM `test` WHERE ( `Y` = MAX( LENGTH( MIN( ? ) ) ) )',
+              args   => ['test', '*', [ Y => { '=' => { -max => { -LENGTH => { -min => 'x' } } } } ] ],
+              stmt   => 'SELECT * FROM test WHERE ( Y = ( MAX( LENGTH( MIN ? ) ) ) )',
+              stmt_q => 'SELECT * FROM `test` WHERE ( `Y` = ( MAX( LENGTH( MIN ? ) ) ) )',
               bind   => [[Y => 'x']],
       },
 );
diff --git a/t/02where.t b/t/02where.t
index d268062..7e9579d 100644
--- a/t/02where.t
+++ b/t/02where.t
@@ -323,16 +323,32 @@ my @handle_tests = (
 
 # Op against random functions (these two are oracle-specific)
    {
-       where => { timestamp => { '!=' => { -trunc => \'sysdate' } } },
-       stmt => " WHERE ( timestamp != TRUNC(sysdate) )",
+       where => { timestamp => { '!=' => { -trunc => { -year => \'sysdate' } } } },
+       stmt => " WHERE ( timestamp != TRUNC (YEAR sysdate) )",
        bind => [],
    },
    {
        where => { timestamp => { '>=' => { -TO_DATE => '2009-12-21 00:00:00' } } },
-       stmt => " WHERE ( timestamp >= TO DATE(?) )",
+       stmt => " WHERE ( timestamp >= TO DATE ? )",
        bind => ['2009-12-21 00:00:00'],
    },
 
+# Legacy function specs
+   {
+       where => { ip => {'<<=' => '127.0.0.1/32' } },
+       stmt => "WHERE ( ip <<= ? )",
+       bind => ['127.0.0.1/32'],
+   },
+   {
+       where => { foo => { 'GLOB' => '*str*' } },
+       stmt => " WHERE foo GLOB ? ",
+       bind => [ '*str*' ],
+   },
+   {
+       where => { foo => { 'REGEXP' => 'bar|baz' } },
+       stmt => " WHERE foo REGEXP ? ",
+       bind => [ 'bar|baz' ],
+   },
 );
 
 plan tests => ( @handle_tests * 2 ) + 1;