Deprecate and properly handle empty lhs

diff --git a/Changes b/Changes
index 99eadea..fb7ba1d 100644 (file)
--- a/Changes
+++ b/Changes
@@ -3,6 +3,7 @@ Revision history for SQL::Abstract
     - New exportable functions: is_literal_value($) and is_plain_value($)
     - New attribute 'escape_char' allowing for proper escape of quote_chars
       present in an identifier
+    - Deprecate { "" => \... } constructs
     - Treat { -value => undef } as plain undef in all cases
     - Explicitly throw on { -ident => undef }
 

diff --git a/lib/SQL/Abstract.pm b/lib/SQL/Abstract.pm
index 5735b4c..a18e493 100644 (file)
--- a/lib/SQL/Abstract.pm
+++ b/lib/SQL/Abstract.pm
@@ -525,7 +525,10 @@ sub _where_ARRAYREF {
 
   my (@sql_clauses, @all_bind);
   # need to use while() so can shift() for pairs
-  while (my $el = shift @clauses) {
+  while (@clauses) {
+    my $el = shift @clauses;
+
+    $el = undef if (defined $el and ! length $el);
 
     # switch according to kind of $el and get corresponding ($sql, @bind)
     my ($sql, @bind) = $self->_SWITCH_refkind($el, {
@@ -543,10 +546,12 @@ sub _where_ARRAYREF {
 
       SCALARREF => sub { ($$el);                                 },
 
-      SCALAR    => sub {# top-level arrayref with scalars, recurse in pairs
-                        $self->_recurse_where({$el => shift(@clauses)})},
+      SCALAR    => sub {
+        # top-level arrayref with scalars, recurse in pairs
+        $self->_recurse_where({$el => shift(@clauses)})
+      },
 
-      UNDEF     => sub {puke "not supported : UNDEF in arrayref" },
+      UNDEF     => sub {puke "Supplying an empty left hand side argument is not supported in array-pairs" },
     });
 
     if ($sql) {
@@ -605,6 +610,15 @@ sub _where_HASHREF {
         ($s, @b);
       }
       else {
+        if (! length $k) {
+          if (is_literal_value ($v) ) {
+            belch 'Hash-pairs consisting of an empty string with a literal are deprecated, and will be removed in 2.0: use -and => [ $literal ] instead';
+          }
+          else {
+            puke "Supplying an empty left hand side argument is not supported in hash-pairs";
+          }
+        }
+
         my $method = $self->_METHOD_FOR_refkind("_where_hashpair", $v);
         $self->$method($k, $v);
       }

diff --git a/t/01generate.t b/t/01generate.t
index 2746b60..c3f83b1 100644 (file)
--- a/t/01generate.t
+++ b/t/01generate.t
@@ -659,6 +659,125 @@ for my $op ( qw(like rlike not_like not_rlike), 'not like', 'not rlike', 'is lik
   } for ('', '-');  # with and without -
 }
 
+# check emtpty-lhs in a hashpair and arraypair
+for my $lhs (undef, '') {
+  no warnings 'uninitialized';
+
+##
+## hard exceptions - never worked
+  for my $where_arg (
+    ( map { $_, { @$_ } }
+      [ $lhs => "foo" ],
+      [ $lhs => { "=" => "bozz" } ],
+      [ $lhs => { "=" => \"bozz" } ],
+      [ $lhs => { -max => \"bizz" } ],
+    ),
+    [ -and => { $lhs => "baz" }, bizz => "buzz" ],
+    [ foo => "bar", { $lhs => "baz" }, bizz => "buzz" ],
+    { foo => "bar", -or => { $lhs => "baz" } },
+
+    # the hashref forms of these work sadly - check for warnings below
+    { foo => "bar", -and => [ $lhs => \"baz" ], bizz => "buzz" },
+    { foo => "bar", -or => [ $lhs => \"baz" ], bizz => "buzz" },
+    [ foo => "bar", [ $lhs => \"baz" ], bizz => "buzz" ],
+    [ foo => "bar", $lhs => \"baz", bizz => "buzz" ],
+    [ foo => "bar", $lhs => \["baz"], bizz => "buzz" ],
+    [ $lhs => \"baz" ],
+    [ $lhs => \["baz"] ],
+
+    # except for this one, that is automagically arrayified
+    { foo => "bar", -or => { $lhs => \"baz" }, bizz => "buzz" },
+  ) {
+    push @tests, {
+      func => 'where',
+      args => [ $where_arg ],
+      throws  => qr/\QSupplying an empty left hand side argument is not supported/,
+    };
+  }
+
+##
+## deprecations - sorta worked, likely abused by folks
+  for my $where_arg (
+    # the arrayref forms of this never worked and throw above
+    { foo => "bar", -and => { $lhs => \"baz" }, bizz => "buzz" },
+    { foo => "bar", $lhs => \"baz", bizz => "buzz" },
+    { foo => "bar", $lhs => \["baz"], bizz => "buzz" },
+  ) {
+    push @tests, {
+      func    => 'where',
+      args    => [ $where_arg ],
+      stmt    => 'WHERE baz AND bizz = ? AND foo = ?',
+      stmt_q  => 'WHERE baz AND `bizz` = ? AND `foo` = ?',
+      bind    => [qw( buzz bar )],
+      warns   => qr/\QHash-pairs consisting of an empty string with a literal are deprecated/,
+    };
+  }
+
+  for my $where_arg (
+    { $lhs => \"baz" },
+    { $lhs => \["baz"] },
+  ) {
+    push @tests, {
+      func    => 'where',
+      args    => [ $where_arg ],
+      stmt    => 'WHERE baz',
+      stmt_q  => 'WHERE baz',
+      bind    => [],
+      warns   => qr/\QHash-pairs consisting of an empty string with a literal are deprecated/,
+    }
+  }
+}
+
+# check false lhs, silly but possible
+{
+  for my $where_arg (
+    [ { 0 => "baz" }, bizz => "buzz", foo => "bar" ],
+    [ -or => { foo => "bar", -or => { 0 => "baz" }, bizz => "buzz" } ],
+  ) {
+    push @tests, {
+      func    => 'where',
+      args    => [ $where_arg ],
+      stmt    => 'WHERE 0 = ? OR bizz = ? OR foo = ?',
+      stmt_q  => 'WHERE `0` = ? OR `bizz` = ? OR `foo` = ?',
+      bind    => [qw( baz buzz bar )],
+    };
+  }
+
+  for my $where_arg (
+    { foo => "bar", -and => [ 0 => \"= baz" ], bizz => "buzz" },
+    { foo => "bar", -or => [ 0 => \"= baz" ], bizz => "buzz" },
+
+    { foo => "bar", -and => { 0 => \"= baz" }, bizz => "buzz" },
+    { foo => "bar", -or => { 0 => \"= baz" }, bizz => "buzz" },
+
+    { foo => "bar", 0 => \"= baz", bizz => "buzz" },
+    { foo => "bar", 0 => \["= baz"], bizz => "buzz" },
+  ) {
+    push @tests, {
+      func    => 'where',
+      args    => [ $where_arg ],
+      stmt    => 'WHERE 0 = baz AND bizz = ? AND foo = ?',
+      stmt_q  => 'WHERE `0` = baz AND `bizz` = ? AND `foo` = ?',
+      bind    => [qw( buzz bar )],
+    };
+  }
+
+  for my $where_arg (
+    [ -and => [ 0 => \"= baz" ], bizz => "buzz", foo => "bar" ],
+    [ -or => [ 0 => \"= baz" ], bizz => "buzz", foo => "bar" ],
+    [ 0 => \"= baz", bizz => "buzz", foo => "bar" ],
+    [ 0 => \["= baz"], bizz => "buzz", foo => "bar" ],
+  ) {
+    push @tests, {
+      func    => 'where',
+      args    => [ $where_arg ],
+      stmt    => 'WHERE 0 = baz OR bizz = ? OR foo = ?',
+      stmt_q  => 'WHERE `0` = baz OR `bizz` = ? OR `foo` = ?',
+      bind    => [qw( buzz bar )],
+    };
+  }
+}
+
 for my $t (@tests) {
   my $new = $t->{new} || {};