From: Guillermo Roditi Date: Thu, 12 Jun 2008 18:34:12 +0000 (+0000) Subject: fix for key => [] + tests + cleanup of 02where.t X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=8a68b5be44b4394787aa83b8b452403b3214e16e;p=scpubgit%2FQ-Branch.git fix for key => [] + tests + cleanup of 02where.t --- diff --git a/Changes b/Changes index 431fc26..1b3d790 100644 --- 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 --- 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 --- 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 => { '>=' => [] }},); +}