From: Peter Rabbitson Date: Wed, 18 Dec 2013 08:48:36 +0000 (+0100) Subject: Backport undef-with-in tests from DQ, add "roadwork ahead" exceptions X-Git-Tag: v1.75~19 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FSQL-Abstract.git;a=commitdiff_plain;h=032dfe204e1d3d8dc43116c8b25ebbca257e9ac0 Backport undef-with-in tests from DQ, add "roadwork ahead" exceptions This is an amalgamation of test changes from 5b67050, 39221d2 and 038b0a7 ilmari++ --- diff --git a/Changes b/Changes index e6488a4..c3d7ddc 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,10 @@ Revision history for SQL::Abstract + - *UPCOMING INCOMPATIBLE BUGFIX*: SQLA used to generate incorrect SQL + on undef-containing lists fed to -in and -not_in. An exception will + be raised for a while before properly fixing this, to avoid quiet + but subtle changes to query results in production + - Fix false negative comparison of ORDER BY ASC - Fix typos in POD and comments (RT#87776) diff --git a/lib/SQL/Abstract.pm b/lib/SQL/Abstract.pm index de10fcd..9e08585 100644 --- a/lib/SQL/Abstract.pm +++ b/lib/SQL/Abstract.pm @@ -1051,7 +1051,12 @@ sub _where_field_IN { $self->_where_unary_op ($1 => $arg); }, UNDEF => sub { - return $self->_sqlcase('null'); + puke( + 'SQL::Abstract before v1.75 used to generate incorrect SQL when the ' + . "-$op operator was given an undef-containing list: !!!AUDIT YOUR CODE " + . 'AND DATA!!! (the upcoming Data::Query-based version of SQL::Abstract ' + . 'will emit the logically correct SQL instead of raising this exception)' + ); }, }); push @all_sql, $sql; diff --git a/t/01generate.t b/t/01generate.t index 3e3cd56..4962c23 100644 --- a/t/01generate.t +++ b/t/01generate.t @@ -552,6 +552,34 @@ my @tests = ( stmt_q => 'SELECT * FROM `test` WHERE ( 0=1 AND 1=1 )', bind => [], }, + { + exception_like => qr/ + \QSQL::Abstract before v1.75 used to generate incorrect SQL \E + \Qwhen the -IN operator was given an undef-containing list: \E + \Q!!!AUDIT YOUR CODE AND DATA!!! (the upcoming Data::Query-based \E + \Qversion of SQL::Abstract will emit the logically correct SQL \E + \Qinstead of raising this exception)\E + /x, + func => 'select', + args => ['test', '*', { a => { -in => [42, undef] }, b => { -not_in => [42, undef] } } ], + stmt => 'SELECT * FROM test WHERE ( ( a IN ( ? ) OR a IS NULL ) AND b NOT IN ( ? ) AND b IS NOT NULL )', + stmt_q => 'SELECT * FROM `test` WHERE ( ( `a` IN ( ? ) OR `a` IS NULL ) AND `b` NOT IN ( ? ) AND `b` IS NOT NULL )', + bind => [ 42, 42 ], + }, + { + exception_like => qr/ + \QSQL::Abstract before v1.75 used to generate incorrect SQL \E + \Qwhen the -IN operator was given an undef-containing list: \E + \Q!!!AUDIT YOUR CODE AND DATA!!! (the upcoming Data::Query-based \E + \Qversion of SQL::Abstract will emit the logically correct SQL \E + \Qinstead of raising this exception)\E + /x, + func => 'select', + args => ['test', '*', { a => { -in => [undef] }, b => { -not_in => [undef] } } ], + stmt => 'SELECT * FROM test WHERE ( a IS NULL AND b IS NOT NULL )', + stmt_q => 'SELECT * FROM `test` WHERE ( `a` IS NULL AND `b` IS NOT NULL )', + bind => [], + }, ); for my $t (@tests) { @@ -591,6 +619,7 @@ for my $t (@tests) { else { $cref->(); } + is_same_sql_bind( $stmt, \@bind, diff --git a/t/05in_between.t b/t/05in_between.t index 12a5658..3ca3c67 100644 --- a/t/05in_between.t +++ b/t/05in_between.t @@ -173,16 +173,30 @@ my @in_between_tests = ( test => '-in with an array of function array refs with args', }, { + exception => qr/ + \QSQL::Abstract before v1.75 used to generate incorrect SQL \E + \Qwhen the -IN operator was given an undef-containing list: \E + \Q!!!AUDIT YOUR CODE AND DATA!!! (the upcoming Data::Query-based \E + \Qversion of SQL::Abstract will emit the logically correct SQL \E + \Qinstead of raising this exception)\E + /x, where => { x => { -in => [ 1, undef ] } }, - stmt => " WHERE ( x IN ( ?, NULL ) )", + stmt => " WHERE ( x IN ( ? ) OR x IS NULL )", bind => [ 1 ], test => '-in with undef as an element', }, { + exception => qr/ + \QSQL::Abstract before v1.75 used to generate incorrect SQL \E + \Qwhen the -IN operator was given an undef-containing list: \E + \Q!!!AUDIT YOUR CODE AND DATA!!! (the upcoming Data::Query-based \E + \Qversion of SQL::Abstract will emit the logically correct SQL \E + \Qinstead of raising this exception)\E + /x, where => { x => { -in => [ 1, undef, 2, 3, undef ] } }, - stmt => " WHERE ( x IN ( ?, NULL, ?, ?, NULL ) )", + stmt => " WHERE ( x IN ( ?, ?, ? ) OR x IS NULL )", bind => [ 1, 2, 3 ], - test => '-in with undef as an element', + test => '-in with multiple undef elements', }, ); @@ -193,24 +207,27 @@ for my $case (@in_between_tests) { local $Data::Dumper::Terse = 1; - lives_ok (sub { + my @w; + local $SIG{__WARN__} = sub { push @w, @_ }; + my $sql = SQL::Abstract->new ($case->{args} || {}); - my @w; - local $SIG{__WARN__} = sub { push @w, @_ }; - my $sql = SQL::Abstract->new ($case->{args} || {}); - lives_ok (sub { + if ($case->{exception}) { + throws_ok { $sql->where($case->{where}) } $case->{exception}; + } + else { + lives_ok { my ($stmt, @bind) = $sql->where($case->{where}); is_same_sql_bind( $stmt, \@bind, $case->{stmt}, $case->{bind}, - ) - || diag "Search term:\n" . Dumper $case->{where}; - }); - is (@w, 0, $case->{test} || 'No warnings within in-between tests') - || diag join "\n", 'Emitted warnings:', @w; - }, "$case->{test} doesn't die"); + ) || diag "Search term:\n" . Dumper $case->{where}; + } "$case->{test} doesn't die"; + } + + is (@w, 0, $case->{test} || 'No warnings within in-between tests') + || diag join "\n", 'Emitted warnings:', @w; } }