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 (file)
--- 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 <function> 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
   }
 }