From dddb9714a7f98ca7f355f90614b321fbee9ab877 Mon Sep 17 00:00:00 2001 From: Haim Dimer Date: Thu, 30 Jul 2026 13:47:23 -0700 Subject: [PATCH 1/2] Escape closing brackets in bracket-quoted identifiers A bracket-quoted identifier whose value contains ] (e.g. [a]]b], value a]b) serialized back to [a]b], which no longer re-parses. Double each ] on display, mirroring the tokenizer folding ]] into ]. Redshift nested quoted identifiers (["a]b"]) store the value as a complete double-quoted string whose inner ] is literal, so those are left unchanged. Fixes #2409 Signed-off-by: Haim Dimer --- src/ast/mod.rs | 14 +++++++++++++- tests/sqlparser_mssql.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 8a9a67a74..b1b088a79 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -385,7 +385,19 @@ impl fmt::Display for Ident { let escaped = value::escape_quoted_string(&self.value, q); write!(f, "{q}{escaped}{q}") } - Some('[') => write!(f, "[{}]", self.value), + Some('[') => { + // Redshift nested quoted identifiers (e.g. `["a]b"]`) store the + // value as a complete double-quoted string whose inner `]` is + // literal, so leave those unchanged. Otherwise double each `]`, + // mirroring the tokenizer folding `]]` into `]`, so the + // identifier round-trips (#2409). + let v = &self.value; + if v.len() >= 2 && v.starts_with('"') && v.ends_with('"') { + write!(f, "[{v}]") + } else { + write!(f, "[{}]", v.replace(']', "]]")) + } + } None => f.write_str(&self.value), _ => panic!("unexpected quote style"), } diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 3faf56f0d..07bfe52fe 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -937,6 +937,18 @@ fn parse_table_name_in_square_brackets() { ); } +#[test] +fn parse_bracket_identifier_with_escaped_closing_bracket() { + // A bracket-quoted identifier whose value contains `]` must serialize + // with the bracket doubled so it round-trips. See #2409. + let select = ms().verified_only_select("SELECT [a]]b]"); + assert_eq!( + &Expr::Identifier(Ident::with_quote('[', "a]b")), + expr_from_projection(&select.projection[0]), + ); + ms().verified_stmt("SELECT [a]]b] FROM [c]]d]"); +} + #[test] fn parse_for_clause() { ms_and_generic().verified_stmt("SELECT a FROM t FOR JSON PATH"); From 409cb2c2ad7af410f9dd6e1de9958a0927f5c7f7 Mon Sep 17 00:00:00 2001 From: Haim Dimer Date: Tue, 4 Aug 2026 18:59:50 -0700 Subject: [PATCH 2/2] Address review: idempotent bracket escaping, no-escape round-trip Escape a lone `]` by doubling it but leave an already-doubled `]]` intact, so the identifier round-trips in the tokenizer's no-escape mode too (previously `[a]]b]` re-serialized to `[a]]]]b]`). The escaper writes straight into the formatter instead of allocating via replace(). Adds a round-trip corpus and a no-escape-mode round-trip test, and drops the issue references from the code comments. --- src/ast/mod.rs | 21 +++++++++++++++------ tests/sqlparser_mssql.rs | 27 ++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index b1b088a79..e5f98b097 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -386,16 +386,25 @@ impl fmt::Display for Ident { write!(f, "{q}{escaped}{q}") } Some('[') => { - // Redshift nested quoted identifiers (e.g. `["a]b"]`) store the - // value as a complete double-quoted string whose inner `]` is - // literal, so leave those unchanged. Otherwise double each `]`, - // mirroring the tokenizer folding `]]` into `]`, so the - // identifier round-trips (#2409). let v = &self.value; if v.len() >= 2 && v.starts_with('"') && v.ends_with('"') { + // A nested double-quoted identifier (e.g. Redshift `["a]b"]`) + // keeps its inner quotes in the value and its `]` is already + // literal, so emit it unchanged. write!(f, "[{v}]") } else { - write!(f, "[{}]", v.replace(']', "]]")) + // Double a lone `]` so the identifier round-trips, but leave + // an already-doubled `]]` intact: in the tokenizer's no-escape + // mode the value still holds the raw `]]`, and re-doubling it + // would corrupt the identifier. + write!(f, "[")?; + let mut rest = v.as_str(); + while let Some(pos) = rest.find(']') { + write!(f, "{}]]", &rest[..pos])?; + let after = &rest[pos + 1..]; + rest = after.strip_prefix(']').unwrap_or(after); + } + write!(f, "{rest}]") } } None => f.write_str(&self.value), diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index 07bfe52fe..3959fc3ff 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -939,14 +939,28 @@ fn parse_table_name_in_square_brackets() { #[test] fn parse_bracket_identifier_with_escaped_closing_bracket() { - // A bracket-quoted identifier whose value contains `]` must serialize - // with the bracket doubled so it round-trips. See #2409. + // A `]` inside a bracket-quoted identifier is escaped by doubling it, so + // `[a]]b]` denotes the identifier `a]b`. let select = ms().verified_only_select("SELECT [a]]b]"); assert_eq!( &Expr::Identifier(Ident::with_quote('[', "a]b")), expr_from_projection(&select.projection[0]), ); - ms().verified_stmt("SELECT [a]]b] FROM [c]]d]"); + + // Round-trips regardless of where the escaped `]` sits. + for sql in [ + "SELECT [a]]b] FROM [c]]d]", + "SELECT []]]", // the identifier is a single `]` + "SELECT [a]]]", // trailing `]` + "SELECT []]b]", // leading `]` + "SELECT [a]]b]]c]", // several escaped `]` + ] { + ms().verified_stmt(sql); + } + + // In no-escape mode the parsed value keeps the raw `]]`, so serializing it + // must not double the brackets again. + ms_no_unescape().verified_stmt("SELECT [a]]b]"); } #[test] @@ -2447,6 +2461,13 @@ fn ms() -> TestedDialects { TestedDialects::new(vec![Box::new(MsSqlDialect {})]) } +fn ms_no_unescape() -> TestedDialects { + TestedDialects::new_with_options( + vec![Box::new(MsSqlDialect {})], + ParserOptions::new().with_unescape(false), + ) +} + // MS SQL dialect with support for optional semi-colon statement delimiters fn tsql() -> TestedDialects { TestedDialects::new_with_options(