Skip to content

Escape every delimiter when displaying quoted identifiers - #2523

Open
robertogallok wants to merge 1 commit into
apache:mainfrom
robertogallok:fix-quoted-ident-display
Open

robertogallok wants to merge 1 commit into
apache:mainfrom
robertogallok:fix-quoted-ident-display

Conversation

@robertogallok

Copy link
Copy Markdown

Ident::Display reuses the string-literal escaper, which guesses whether a delimiter inside the value is already escaped. An Ident value is always the decoded name, so the guess corrupts identifiers whose name contains two adjacent delimiters or a backslash before one:

let ast = Parser::parse_sql(&GenericDialect {}, r#"SELECT "a""""b""#).unwrap(); // identifier a""b
println!("{}", ast[0]);   // SELECT "a""b"   -> parses back as the identifier a"b

The same happens with backticks (`a````b`) and with a value such as a\"b, which displays as "a\"b" and no longer parses.

The fix doubles every delimiter unconditionally, streaming into the formatter without allocating. Bracket identifiers are left untouched because #2418 already covers them.

with_unescape(false) keeps the raw doubled delimiters in the value, and one Display cannot serve both readings of the same value. This change chooses the default mode, so the two MySQL tests that asserted a display round trip in no-escape mode now assert only the parsed AST.

Tests: a round trip for consecutive delimiters in " and ` identifiers, plus an exhaustive round trip over every value of length 1..=4 built from the delimiter, a backslash, an ASCII and a multi-byte character, across all dialects that delimit identifiers with that character. The exhaustive test fails on main (smallest failing value: "").

AI assistance was used to draft this change; I reviewed and ran the tests, cargo fmt and cargo clippy.

The tokenizer decodes doubled delimiters, so an Ident value holds the
literal name. Ident::Display reused the string-literal escaper, whose
heuristic leaves adjacent delimiters and backslash-preceded delimiters
untouched; a value such as a""b displayed as "a""b", which parses as a
different identifier. Double each delimiter unconditionally, without
allocating. The two MySQL tests that asserted a display round trip in
no-escape mode now assert only the parsed AST, since a value that keeps
its raw escapes cannot share one Display with a decoded one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants