Skip to content

Commit 72277f8

Browse files
def-claude
andauthored
sql: Fix pretty/display round-trip bugs found by fuzzing (#36983)
Print→reparse round-trip bugs in the SQL parser and pretty-printer, surfaced by the grammar-aware fuzz target. Each fix has a regression test; the sqllogictest/testdrive plan goldens are refreshed to match. Themes: quoting bare keyword identifiers (any/all/some/list, context-sensitive keywords), parenthesizing low-precedence operands (prefix ops, casts, COLLATE, quantified comparisons), special-form display correctness (EXTRACT/POSITION/SUBSCRIBE), and bounding parser recursion/backtracking to reject pathological inputs. Found by the cargo-fuzz suite ([separate infra PR](#36982)). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3dcf15e commit 72277f8

27 files changed

Lines changed: 1734 additions & 83 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/expr/src/explain/text.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,6 +1197,14 @@ impl HumanizerMode for HumanizedExplain {
11971197
fn humanize_ident(col: usize, ident: Ident, f: &mut fmt::Formatter<'_>) -> fmt::Result {
11981198
if ident.as_str() == UNKNOWN_COLUMN_NAME {
11991199
write!(f, "#{col}")
1200+
} else if ident.has_only_bare_chars() {
1201+
// The leading `#c` already disambiguates the column, and EXPLAIN
1202+
// output is never reparsed, so a keyword-named column (`any`,
1203+
// `all`, …) needs no quoting here — unlike in SQL display. Print
1204+
// the name bare for legibility; only fall back to the quoting
1205+
// `Ident` display for names with awkward characters (whitespace,
1206+
// braces, quotes) that would otherwise muddle the `{…}` annotation.
1207+
write!(f, "#{col}{{{}}}", ident.as_str())
12001208
} else {
12011209
write!(f, "#{col}{{{ident}}}")
12021210
}

src/sql-lexer/src/keywords.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,19 @@ impl Keyword {
7474
)
7575
}
7676

77+
/// Reports whether this keyword begins a query body (`SELECT`, `VALUES`,
78+
/// `TABLE …`, etc.).
79+
///
80+
/// When `AstDisplay` parenthesizes an expression (e.g. to disambiguate a
81+
/// field access like `(expr).f`) and that expression's leading token is a
82+
/// bare identifier with one of these names, the re-parser treats the
83+
/// parentheses as a subquery and the identifier as its leading clause
84+
/// (e.g. `(table & x)` parses as a `TABLE`-query). Such identifiers must be
85+
/// quoted to round-trip. `SELECT`/`WITH` are already `is_always_reserved`.
86+
pub fn begins_query_body(self) -> bool {
87+
matches!(self, WITH | SELECT | VALUES | SHOW | TABLE)
88+
}
89+
7790
/// Reports whether this keyword requires quoting when used in scalar expressions.
7891
///
7992
/// These are the keywords `Parser::parse_prefix` won't parse as an identifier.
@@ -140,6 +153,33 @@ impl Keyword {
140153
|| self.is_reserved_in_column_alias()
141154
|| self.is_reserved_in_scalar_expression()
142155
}
156+
157+
/// Reports whether a keyword has a special parser-dispatch form (e.g.
158+
/// `POSITION(expr IN expr)`, `MAP[K => V]`) such that an unquoted
159+
/// occurrence in expression position triggers the special grammar
160+
/// rather than parsing as a plain identifier. The parser itself
161+
/// disambiguates by looking at the next token, but `AstDisplay` has no
162+
/// such context — so when emitting an `Ident` whose name matches one
163+
/// of these, we force quoting to keep the round trip stable.
164+
pub fn is_context_sensitive_keyword(self) -> bool {
165+
matches!(
166+
self,
167+
ALL | ANY
168+
| COALESCE
169+
| EXISTS
170+
| EXTRACT
171+
| GREATEST
172+
| LEAST
173+
| MAP
174+
| NORMALIZE
175+
| NULLIF
176+
| POSITION
177+
| ROW
178+
| SOME
179+
| SUBSTRING
180+
| TRIM
181+
)
182+
}
143183
}
144184

145185
impl FromStr for Keyword {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ impl AstDisplay for CsrSeedProtobufSchema {
429429
f.write_str("SCHEMA '");
430430
f.write_str(&display::escape_single_quote_string(&self.schema));
431431
f.write_str("' MESSAGE '");
432-
f.write_str(&self.message_name);
432+
f.write_str(&display::escape_single_quote_string(&self.message_name));
433433
f.write_str("'");
434434
}
435435
}

0 commit comments

Comments
 (0)