From df11609c57ce23e4e020830d7fbc18c7e35bce43 Mon Sep 17 00:00:00 2001 From: Jon Currey Date: Mon, 12 Oct 2026 02:28:28 -0400 Subject: [PATCH] sql-parser: quote bracketed ids that need it on display The `[ AS ]` item name and the `[]` cluster and network policy names only accept an identifier token for the id (the latter two also a number token). Display printed a keyword id like `row` bare (`RawItemName`), or printed the id with no quoting at all (`RawClusterName`, `RawNetworkPolicyName`), so the output failed to reparse ("expected id"). Route all three through one helper that prints the id bare only when it lexes back as the same token, and force-quotes it otherwise. Found by the sql_roundtrip cargo-fuzz target (SQL-769). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/sql-parser/src/ast/metadata.rs | 40 ++++++++++++++++++++---------- src/sql-parser/tests/testdata/id | 39 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/sql-parser/src/ast/metadata.rs b/src/sql-parser/src/ast/metadata.rs index 2a4284d959ddf..1f23fe07addb3 100644 --- a/src/sql-parser/src/ast/metadata.rs +++ b/src/sql-parser/src/ast/metadata.rs @@ -107,23 +107,33 @@ impl RawItemName { } } +/// Writes the id of a bracketed raw name (`[ AS …]`, `[]`) so that it +/// reparses to the same string. +/// +/// The parser takes only an identifier token there (and, when `allow_number`, +/// a number token), so a crafted id with spaces or any keyword (e.g. `row`, +/// which lexes as a keyword token) must be quoted. Any keyword counts, not +/// only the reserved ones `Ident::can_be_printed_bare` checks. A normal global +/// id like `u1` is printed bare in every mode — including the stable mode +/// `pg_get_viewdef` renders in, which would otherwise force-quote it. +fn write_bracketed_id(f: &mut AstFormatter, id: &str, allow_number: bool) { + let ident = Ident::new_unchecked(id); + let is_number = !id.is_empty() && id.chars().all(|ch| ch.is_ascii_digit()); + if (ident.has_only_bare_chars() && ident.as_keyword().is_none()) || (allow_number && is_number) + { + f.write_str(id); + } else { + f.write_str(ident.to_ast_string_stable()); + } +} + impl AstDisplay for RawItemName { fn fmt(&self, f: &mut AstFormatter) { match self { RawItemName::Name(o) => f.write_node(o), RawItemName::Id(id, o, v) => { - // `id` is parsed from an identifier token (`[ AS …]`), so a - // crafted id with spaces/keywords must be quoted to reparse as a - // single identifier. A normal global id like `u1` is printed - // bare in every mode — including the stable mode `pg_get_viewdef` - // renders in, which would otherwise force-quote it. f.write_str("["); - let id = Ident::new_unchecked(id.clone()); - if id.can_be_printed_bare() { - f.write_str(id.as_str()); - } else { - f.write_node(&id); - } + write_bracketed_id(f, id, false); f.write_str(" AS "); f.write_node(o); if let Some(v) = v { @@ -162,7 +172,9 @@ impl AstDisplay for RawClusterName { match self { RawClusterName::Unresolved(id) => f.write_node(id), RawClusterName::Resolved(id) => { - f.write_str(format!("[{}]", id)); + f.write_str("["); + write_bracketed_id(f, id, true); + f.write_str("]"); } } } @@ -194,7 +206,9 @@ impl AstDisplay for RawNetworkPolicyName { match self { RawNetworkPolicyName::Unresolved(id) => f.write_node(id), RawNetworkPolicyName::Resolved(id) => { - f.write_str(format!("[{}]", id)); + f.write_str("["); + write_bracketed_id(f, id, true); + f.write_str("]"); } } } diff --git a/src/sql-parser/tests/testdata/id b/src/sql-parser/tests/testdata/id index f73e3f55197d4..928e9b3de0ee0 100644 --- a/src/sql-parser/tests/testdata/id +++ b/src/sql-parser/tests/testdata/id @@ -87,3 +87,42 @@ SELECT * FROM [ü1 AS materialize.public.foo] SELECT * FROM ["ü1" AS materialize.public.foo] => Select(SelectStatement { query: Query { ctes: Simple([]), body: Select(Select { distinct: None, projection: [Wildcard], from: [TableWithJoins { relation: Table { name: Id("ü1", UnresolvedItemName([Ident("materialize"), Ident("public"), Ident("foo")]), None), alias: None }, joins: [] }], selection: None, group_by: [], having: None, qualify: None, options: [] }), order_by: [], limit: None, offset: None }, as_of: None }) + +# An id that is a keyword must stay quoted when displayed: `[ AS …]` only +# accepts an identifier token, so a bare keyword id fails to reparse +# (SQL-769). Applies to every position that takes a raw item name, including a +# cast's type. +parse-statement +SELECT * FROM ["row" AS materialize.public.foo] +---- +SELECT * FROM ["row" AS materialize.public.foo] +=> +Select(SelectStatement { query: Query { ctes: Simple([]), body: Select(Select { distinct: None, projection: [Wildcard], from: [TableWithJoins { relation: Table { name: Id("row", UnresolvedItemName([Ident("materialize"), Ident("public"), Ident("foo")]), None), alias: None }, joins: [] }], selection: None, group_by: [], having: None, qualify: None, options: [] }), order_by: [], limit: None, offset: None }, as_of: None }) + +parse-statement +SELECT x::["row" AS materialize.public.foo] +---- +SELECT x::["row" AS materialize.public.foo] +=> +Select(SelectStatement { query: Query { ctes: Simple([]), body: Select(Select { distinct: None, projection: [Expr { expr: Cast { expr: Identifier([Ident("x")]), data_type: Other { name: Id("row", UnresolvedItemName([Ident("materialize"), Ident("public"), Ident("foo")]), None), typ_mod: [] } }, alias: None }], from: [], selection: None, group_by: [], having: None, qualify: None, options: [] }), order_by: [], limit: None, offset: None }, as_of: None }) + +parse-statement roundtrip +SELECT listbar::["row" AS linger.over]::[aabortkafka AS linger.over] = cte +---- +SELECT listbar::["row" AS linger.over]::[aabortkafka AS linger.over] = cte + +# The same applies to the `[]` resolved cluster and network policy names, +# which accept an identifier or a number token. A numeric id stays bare. +parse-statement +CREATE INDEX i IN CLUSTER ["row"] ON t (a) +---- +CREATE INDEX i IN CLUSTER ["row"] ON t (a) +=> +CreateIndex(CreateIndexStatement { name: Some(Ident("i")), in_cluster: Some(Resolved("row")), on_name: Name(UnresolvedItemName([Ident("t")])), key_parts: Some([Identifier([Ident("a")])]), with_options: [], if_not_exists: false }) + +parse-statement +COMMENT ON NETWORK POLICY ["row"] IS 'x' +---- +COMMENT ON NETWORK POLICY ["row"] IS 'x' +=> +Comment(CommentStatement { object: NetworkPolicy { name: Resolved("row") }, comment: Some("x") })