From: Peter Rabbitson Date: Fri, 3 Jan 2014 16:57:16 +0000 (+0100) Subject: Ensure that multi-nested parenthesis are never unrolled after IN X-Git-Tag: v1.77~5 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7d27345242d34aa0ad38e3a5f54d0bd09b558d32;p=dbsrgits%2FSQL-Abstract.git Ensure that multi-nested parenthesis are never unrolled after IN This lets us remove parenthesis_significant from many tests that do not need it, including reversal of 1ba9d0f03 --- diff --git a/lib/SQL/Abstract/Tree.pm b/lib/SQL/Abstract/Tree.pm index 519cae4..c6faef9 100644 --- a/lib/SQL/Abstract/Tree.pm +++ b/lib/SQL/Abstract/Tree.pm @@ -659,7 +659,7 @@ sub _parenthesis_unroll { } # unroll nested parenthesis - while ( @{$child->[1]} == 1 and $child->[1][0][0] eq '-PAREN') { + while ( $ast->[0] ne 'IN' and @{$child->[1]} == 1 and $child->[1][0][0] eq '-PAREN') { $child = $child->[1][0]; $changes++; } diff --git a/t/05in_between.t b/t/05in_between.t index d3ede2f..832b7a1 100644 --- a/t/05in_between.t +++ b/t/05in_between.t @@ -141,46 +141,40 @@ my @in_between_tests = ( }, { - parenthesis_significant => 1, where => { x => { -in => [ 1 .. 3] } }, - stmt => "WHERE ( x IN (?, ?, ?) )", - bind => [ 1 .. 3], + stmt => "WHERE x IN (?, ?, ?)", + bind => [ 1 .. 3 ], test => '-in with an array of scalars', }, { - parenthesis_significant => 1, where => { x => { -in => [] } }, - stmt => "WHERE ( 0=1 )", + stmt => "WHERE 0=1", bind => [], test => '-in with an empty array', }, { - parenthesis_significant => 1, where => { x => { -in => \'( 1,2,lower(y) )' } }, - stmt => "WHERE ( x IN ( 1,2,lower(y) ) )", + stmt => "WHERE x IN ( 1,2,lower(y) )", bind => [], test => '-in with a literal scalarref', }, # note that outer parens are opened even though literal was requested below { - parenthesis_significant => 1, where => { x => { -in => \['( ( ?,?,lower(y) ) )', 1, 2] } }, - stmt => "WHERE ( x IN ( ?,?,lower(y) ) )", + stmt => "WHERE x IN ( ?,?,lower(y) )", bind => [1, 2], test => '-in with a literal arrayrefref', }, { - parenthesis_significant => 1, where => { status => { -in => \"(SELECT status_codes\nFROM states)" }, }, - stmt => " WHERE ( status IN ( SELECT status_codes FROM states )) ", + stmt => " WHERE status IN ( SELECT status_codes FROM states )", bind => [], test => '-in multi-line subquery test', }, { - parenthesis_significant => 1, where => { customer => { -in => \[ 'SELECT cust_id FROM cust WHERE balance > ?', @@ -189,17 +183,15 @@ my @in_between_tests = ( status => { -in => \'SELECT status_codes FROM states' }, }, stmt => " - WHERE (( + WHERE customer IN ( SELECT cust_id FROM cust WHERE balance > ? ) AND status IN ( SELECT status_codes FROM states ) - )) ", bind => [2000], test => '-in POD test', }, { - parenthesis_significant => 1, where => { x => { -in => [ \['LOWER(?)', 'A' ], \'LOWER(b)', { -lower => 'c' } ] } }, stmt => " WHERE ( x IN ( LOWER(?), LOWER(b), LOWER ? ) )", bind => [qw/A c/], @@ -213,7 +205,6 @@ my @in_between_tests = ( \Qversion of SQL::Abstract will emit the logically correct SQL \E \Qinstead of raising this exception)\E /x, - parenthesis_significant => 1, where => { x => { -in => [ 1, undef ] } }, stmt => " WHERE ( x IN ( ? ) OR x IS NULL )", bind => [ 1 ], @@ -227,16 +218,14 @@ my @in_between_tests = ( \Qversion of SQL::Abstract will emit the logically correct SQL \E \Qinstead of raising this exception)\E /x, - parenthesis_significant => 1, where => { x => { -in => [ 1, undef, 2, 3, undef ] } }, stmt => " WHERE ( x IN ( ?, ?, ? ) OR x IS NULL )", bind => [ 1, 2, 3 ], test => '-in with multiple undef elements', }, { - parenthesis_significant => 1, where => { a => { -in => 42 }, b => { -not_in => 42 } }, - stmt => ' WHERE ( ( a IN ( ? ) AND b NOT IN ( ? ) ) )', + stmt => ' WHERE a IN ( ? ) AND b NOT IN ( ? )', bind => [ 42, 42 ], test => '-in, -not_in with scalar', }, @@ -254,7 +243,6 @@ my @in_between_tests = ( \Qversion of SQL::Abstract will emit the logically correct SQL \E \Qinstead of raising this exception)\E /x, - parenthesis_significant => 1, where => { a => { -in => [42, undef] }, b => { -not_in => [42, undef] } }, stmt => ' WHERE ( ( a IN ( ? ) OR a IS NULL ) AND b NOT IN ( ? ) AND b IS NOT NULL )', bind => [ 42, 42 ], diff --git a/t/10test.t b/t/10test.t index 8d02f4e..60d10d7 100644 --- a/t/10test.t +++ b/t/10test.t @@ -257,10 +257,10 @@ my @sql_tests = ( }, { equal => 0, - opts => { parenthesis_significant => 1 }, statements => [ - q/SELECT foo FROM bar WHERE a IN (1,2,3)/, q/SELECT foo FROM bar WHERE a IN (1,3,2)/, + q/SELECT foo FROM bar WHERE a IN 1,2,3/, + q/SELECT foo FROM bar WHERE a IN (1,2,3)/, q/SELECT foo FROM bar WHERE a IN ((1,2,3))/, ] }, diff --git a/t/11parser.t b/t/11parser.t index 7cf6509..3c60c95 100644 --- a/t/11parser.t +++ b/t/11parser.t @@ -811,7 +811,7 @@ is_deeply($sqlat->parse('SELECT foo FROM bar ORDER BY x + ? DESC, oomph, y - ? D ] ], 'Crazy ORDER BY parsed correctly'); -is_deeply( $sqlat->parse("META SELECT * * FROM (SELECT *, FROM foobar baz buzz) foo bar WHERE NOT NOT NOT EXISTS (SELECT 'cr,ap') AND foo.a = ? STUFF moar(stuff) and not (foo.b LIKE 'station') and x = y and a = b and GROUP BY , ORDER BY x x1 x2 y asc, max(y) desc x z desc"), [ +is_deeply( $sqlat->parse("META SELECT * * FROM (SELECT *, FROM foobar baz buzz) foo bar WHERE NOT NOT NOT EXISTS (SELECT 'cr,ap') AND foo.a = ? STUFF moar(stuff) and not (foo.b LIKE 'station') and x = y and z in ((1, 2)) and a = b and GROUP BY , ORDER BY x x1 x2 y asc, max(y) desc x z desc"), [ [ "-LITERAL", [ @@ -1047,6 +1047,44 @@ is_deeply( $sqlat->parse("META SELECT * * FROM (SELECT *, FROM foobar baz buzz) ] ], [ + 'IN', + [ + [ + '-LITERAL', + [ + 'z', + ], + ], + [ + '-PAREN', + [ + [ + '-PAREN', + [ + [ + '-LIST', + [ + [ + '-LITERAL', + [ + '1' + ] + ], + [ + '-LITERAL', + [ + '2' + ] + ], + ], + ], + ], + ], + ], + ], + ], + ], + [ "=", [ [ diff --git a/t/14roundtrippin.t b/t/14roundtrippin.t index 981297e..9dc581e 100644 --- a/t/14roundtrippin.t +++ b/t/14roundtrippin.t @@ -53,8 +53,8 @@ for my $orig (@sql) { $sqlat->unparse($ast); }; - # deal with parenthesis readjustment - $_ =~ s/\s*([\(\)])\s*/$1 /g + # deal with whitespace around parenthesis readjustment + $_ =~ s/ \s* ( [ \(\) ] ) \s* /$1/gx for ($orig, $reassembled); is ( @@ -69,6 +69,15 @@ for my $orig (@sql) { }; } +# this is invalid SQL, we are just checking that the parser +# does not inadvertently make it right +my $sql = 'SELECT * FROM foo WHERE x IN ( ( 1 ) )'; +is( + $sqlat->unparse($sqlat->parse($sql)), + $sql, + 'Multi-parens around IN survive', +); + lives_ok { $sqlat->unparse( $sqlat->parse( <<'EOS' ) ) } 'Able to parse/unparse grossly malformed sql'; SELECT (