Skip to content

Commit e4e1fbf

Browse files
def-claude
andcommitted
sql: parenthesize a LIKE pattern whose right spine would steal ESCAPE
A `[I]LIKE` pattern parses at `Like` precedence and, when an `ESCAPE` follows, sits immediately to the left of the `ESCAPE` keyword. Only `[I]LIKE` consumes `ESCAPE`, so a `[I]LIKE` exposed on the pattern's *right* spine steals the outer escape as its own: `x LIKE NOT (a LIKE b) ESCAPE c`, printed bare as `x LIKE NOT a LIKE b ESCAPE c`, binds the escape onto the inner `a LIKE b` instead of the outer `LIKE`. The pattern's left edge was already guarded (so its left spine doesn't re-associate out of the `LIKE`), but the escape-stealing hazard is on the right spine and only exists when an escape is present. Add a right-edge guard conditioned on `escape.is_some()`. The parser never builds a pattern that is itself a trailing escapeless `[I]LIKE` under an outer escape (it left-nests instead), so parser-produced ASTs are unaffected and the datadriven suites are unchanged. This drift collides under stable-string printing (both ASTs print to the same text), so the round-trip fuzz oracle was strengthened to compare reparsed ASTs structurally rather than by re-printed string. Tests: adds `like_pattern_reparenthesized_after_nested_stripped` to `sqlparser_common.rs`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3f73b8a commit e4e1fbf

2 files changed

Lines changed: 62 additions & 6 deletions

File tree

src/sql-parser/src/ast/defs/expr.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,18 @@ impl<T: AstInfo> AstDisplay for Expr<T> {
302302
f.write_str("I");
303303
}
304304
f.write_str("LIKE ");
305-
// The pattern and escape parse at `Like` precedence and sit to
306-
// the right of the keyword, so an operand exposing a precedence at
307-
// or below `Like` on its left spine (e.g. an `IN`/`LIKE`/`BETWEEN`
308-
// at equal precedence) re-associates unless parenthesized —
309-
// `a LIKE b IN (q)` parses as `(a LIKE b) IN (q)`.
310-
write_binary_operand(f, pattern, left_edge(pattern) <= prec::LIKE);
305+
// The pattern and escape parse at `Like` precedence and sit to the
306+
// right of the keyword, so an operand exposing a precedence at or
307+
// below `Like` on its left spine (e.g. an `IN`/`LIKE`/`BETWEEN` at
308+
// equal precedence) re-associates unless parenthesized —
309+
// `a LIKE b IN (q)` parses as `(a LIKE b) IN (q)`. When an `ESCAPE`
310+
// follows, the pattern is *also* immediately left of `ESCAPE`: a
311+
// `[I]LIKE` exposed on the pattern's right spine would steal the
312+
// `ESCAPE` as its own (`a LIKE NOT b LIKE c ESCAPE d` parses the
313+
// escape onto the inner `b LIKE c`), so guard the right edge too.
314+
let pattern_parens = left_edge(pattern) <= prec::LIKE
315+
|| (escape.is_some() && right_edge(pattern) <= prec::LIKE);
316+
write_binary_operand(f, pattern, pattern_parens);
311317
if let Some(escape) = escape {
312318
f.write_str(" ESCAPE ");
313319
write_binary_operand(f, escape, left_edge(escape) <= prec::LIKE);

src/sql-parser/tests/sqlparser_common.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,3 +940,53 @@ fn is_distinct_from_rhs_reparenthesized_after_nested_stripped() {
940940
);
941941
}
942942
}
943+
944+
#[mz_ore::test]
945+
fn like_pattern_reparenthesized_after_nested_stripped() {
946+
use mz_sql_parser::ast::display::AstDisplay;
947+
use mz_sql_parser::ast::visit_mut::{self, VisitMut};
948+
use mz_sql_parser::ast::{AstInfo, Expr};
949+
950+
struct StripNested;
951+
impl<'a, T: AstInfo> VisitMut<'a, T> for StripNested {
952+
fn visit_expr_mut(&mut self, e: &'a mut Expr<T>) {
953+
visit_mut::visit_expr_mut(self, e);
954+
if let Expr::Nested(inner) = e {
955+
*e = (**inner).clone();
956+
}
957+
}
958+
}
959+
960+
// A `[I]LIKE` pattern parses at `Like` precedence. When an `ESCAPE` follows,
961+
// the pattern is immediately left of the `ESCAPE` keyword, so a `[I]LIKE`
962+
// exposed on the pattern's *right* spine steals the `ESCAPE` as its own:
963+
// `x LIKE NOT (a LIKE b) ESCAPE c`, printed bare as
964+
// `x LIKE NOT a LIKE b ESCAPE c`, binds the escape to the inner `a LIKE b`.
965+
// The drift is structural and produces colliding stable strings, so it is
966+
// checked structurally.
967+
for sql in [
968+
"SELECT x LIKE (NOT (a LIKE b)) ESCAPE c",
969+
"SELECT x LIKE (a LIKE b) ESCAPE c",
970+
"SELECT x ILIKE (a ILIKE b) ESCAPE c",
971+
// Left-spine cases (covered with or without escape).
972+
"SELECT x LIKE (a IN (SELECT b FROM t))",
973+
// No escape: a right-spine LIKE is harmless, so this stays bare.
974+
"SELECT x LIKE NOT (a LIKE b)",
975+
] {
976+
let mut ast = mz_sql_parser::parser::parse_statements(sql)
977+
.unwrap()
978+
.remove(0)
979+
.ast;
980+
StripNested.visit_statement_mut(&mut ast);
981+
let displayed = ast.to_ast_string_simple();
982+
let mut reparsed = mz_sql_parser::parser::parse_statements(&displayed)
983+
.unwrap()
984+
.remove(0)
985+
.ast;
986+
StripNested.visit_statement_mut(&mut reparsed);
987+
assert_eq!(
988+
ast, reparsed,
989+
"LIKE pattern display did not round-trip after Nested was stripped: {sql:?} -> {displayed:?}"
990+
);
991+
}
992+
}

0 commit comments

Comments
 (0)