fix for key => [] + tests + cleanup of 02where.t

diff --git a/Changes b/Changes
index 431fc26..1b3d790 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,7 @@
 Revision history for SQL::Abstract
 
+    - Make col => [] and col => {$op => [] } DTRT or die instead of generating
+      broken SQL. Added tests for this. 
     - Added { -desc => 'column' } order by support (Ash)
     - Tiny "$_"-related fix for { -desc => 'columns'} order by support 
       - tests + docs (groditi)

diff --git a/lib/SQL/Abstract.pm b/lib/SQL/Abstract.pm
index 6816a52..ba798ac 100644 (file)
--- a/lib/SQL/Abstract.pm
+++ b/lib/SQL/Abstract.pm
@@ -744,25 +744,29 @@ sub _recurse_where {
                 $self->_debug("UNDEF($k) means IS NULL");
                 push @sqlf, $label . $self->_sqlcase(' is null');
             } elsif (ref $v eq 'ARRAY') {
-                my @v = @$v;
-                
-                # multiple elements: multiple options
-                $self->_debug("ARRAY($k) means multiple elements: [ @v ]");
-
-                # special nesting, like -and, -or, -nest, so shift over
-                my $subjoin = $self->_sqlcase('or');
-                if ($v[0] =~ /^-(\D+)/) {
-                    $subjoin = $self->_modlogic($1);    # override subjoin
-                    $self->_debug("OP(-$1) means special logic ($subjoin), shifting...");
-                    shift @v;
-                }
+                if( @$v ) {
+                    my @v = @$v;
+                    # multiple elements: multiple options
+                    $self->_debug("ARRAY($k) means multiple elements: [ @v ]");
+
+                    # special nesting, like -and, -or, -nest, so shift over
+                    my $subjoin = $self->_sqlcase('or');
+                    if ($v[0] =~ /^-(\D+)/) {
+                        $subjoin = $self->_modlogic($1);    # override subjoin
+                        $self->_debug("OP(-$1) means special logic ($subjoin), shifting...");
+                        shift @v;
+                    }
 
-                # map into an array of hashrefs and recurse
-                my @ret = $self->_recurse_where([map { {$k => $_} } @v], $subjoin);
+                    # map into an array of hashrefs and recurse
+                    my @ret = $self->_recurse_where([map { {$k =>  $_} } @v], $subjoin);
 
-                # push results into our structure
-                push @sqlf, shift @ret;
-                push @sqlv, @ret;
+                    # push results into our structure
+                    push @sqlf, shift @ret;
+                    push @sqlv, @ret;
+                } else {
+                    $self->_debug("empty ARRAY($k) means 0=1");
+                    push @sqlf, '0=1';
+                }
             } elsif (ref $v eq 'HASH') {
                 # modified operator { '!=', 'completed' }
                 for my $f (sort keys %$v) {
@@ -795,16 +799,25 @@ sub _recurse_where {
                                   push(@sqlf, ($u =~ /not/i ? "1=1" : "0=1"));
                               }
                               push @sqlv, $self->_bindtype($k, @$x);
+                          } elsif(@$x) {
+                                # multiple elements: multiple options
+                                $self->_debug("ARRAY($x) means multiple elements: [ @$x ]");
+                                # map into an array of hashrefs and recurse
+                                my @ret = $self->_recurse_where([map { {$k => {$f, $_}} } @$x]);
+
+                                # push results into our structure
+                                push @sqlf, shift @ret;
+                                push @sqlv, @ret;
                           } else {
-                              # multiple elements: multiple options
-                              $self->_debug("ARRAY($x) means multiple elements: [ @$x ]");
-                              
-                              # map into an array of hashrefs and recurse
-                              my @ret = $self->_recurse_where([map { {$k => {$f, $_}} } @$x]);
-                              
-                              # push results into our structure
-                              push @sqlf, shift @ret;
-                              push @sqlv, @ret;
+                              #DTRT for $op => []
+                              # I feel like <= and >= should resolve to 0=1 but I am not sure.
+                              if($f eq '='){
+                                  push @sqlf, '0=1';
+                              } elsif( $f eq '!='){
+                                  push @sqlf, '1=1';
+                              } else {
+                                  $self->puke("Can not generate SQL for '${f}' comparison of '${k}' using empty array");
+                              }
                           }
                     } elsif (! defined($x)) {
                         # undef = NOT null
@@ -1061,6 +1074,9 @@ This simple code will create the following:
     $stmt = "WHERE user = ? AND ( status = ? OR status = ? OR status = ? )";
     @bind = ('nwiger', 'assigned', 'in-progress', 'pending');
 
+Please note that an empty arrayref will be considered a logical false and
+will generate 0=1.
+
 If you want to specify a different type of operator for your comparison,
 you can use a hashref for a given column:
 
@@ -1078,6 +1094,9 @@ To test against multiple values, just enclose the values in an arrayref:
 
     status => { '!=', ['assigned', 'in-progress', 'pending'] };
 
+An empty arrayref will try to Do The Right Thing for the operators '=', '!=',
+'-in' '-not_in', but will throw an exception for everything else.
+
 Which would give you:
 
     "WHERE status != ? OR status != ? OR status != ?"

diff --git a/t/02where.t b/t/02where.t
index 03fbdc7..7c36091 100755 (executable)
--- a/t/02where.t
+++ b/t/02where.t
@@ -3,8 +3,9 @@
 use strict;
 use warnings;
 use Test::More;
+use Test::Exception;
 
-plan tests => 24;
+plan tests => 27;
 
 use SQL::Abstract;
 
@@ -146,26 +147,25 @@ my @handle_tests = (
         bind => [7,8,9,'a','b',100,200,150,160,'zz','yy','30','40'],
     },
 
+    {
+        where => {
+            id  => [],
+            bar => {'!=' => []},
+        },
+        stmt => " WHERE ( 1=1 AND 0=1 )",
+        bind => [],
+    },
+
 );
 
-for (@handle_tests) {
-    local $" = ', ';
-    #print "creating a handle with args ($_->{args}): ";
+for my $case (@handle_tests) {
     my $sql = SQL::Abstract->new;
-
-    # run twice
-    for (my $i=0; $i < 2; $i++) {
-        my($stmt, @bind) = $sql->where($_->{where}, $_->{order});
-        my $bad = 0;
-        for(my $i=0; $i < @{$_->{bind}}; $i++) {
-            $bad++ unless $_->{bind}[$i] eq $bind[$i];
-        }
-
-        ok($stmt eq $_->{stmt} && @bind == @{$_->{bind}} && ! $bad) or 
-                print "got\n",
-                      "[$stmt] [@bind]\n",
-                      "instead of\n",
-                      "[$_->{stmt}] [@{$_->{bind}}]\n\n";
-    }
+    my($stmt, @bind) = $sql->where($case->{where}, $case->{order});
+    is($stmt, $case->{stmt});
+    is_deeply(\@bind, $case->{bind});
 }
 
+dies_ok {
+    my $sql = SQL::Abstract->new;
+    $sql->where({ foo => { '>=' => [] }},);
+}