From: Peter Rabbitson Date: Thu, 25 Sep 2014 11:08:35 +0000 (+0200) Subject: Deprecate and properly handle empty lhs X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=scpubgit%2FQ-Branch.git;a=commitdiff_plain;h=b5a576d25b88be59b34696bedd5fcd217aed7aae Deprecate and properly handle empty lhs --- diff --git a/Changes b/Changes index 99eadea..fb7ba1d 100644 --- 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 --- 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 --- 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} || {};