From df157671780988cec97f695938a044cadf2cef23 Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Fri, 31 Jul 2026 19:02:28 +0000 Subject: [PATCH] sql: quote a bare `map` identifier so option values round-trip `parse_option_value` accepts a `MAP[k => v]` literal, and that grammar commits to the map form on the `MAP` keyword alone and then demands `[`. Expression position dispatches on a two-token lookahead (`MAP` followed by `[` or `(`) and otherwise falls through to an identifier, but the option-value grammar has no such fallback. So an option value named `map` printed bare, which both `AstDisplay` and the pretty printer did, fails to reparse: CREATE SINK s FROM t INTO KAFKA CONNECTION c (TOPIC = "map") FORMAT BYTES ENVELOPE DEBEZIUM printed `TOPIC = map`, and reparsing that raised "Expected left square bracket, found right parenthesis". Every generic option value is affected, a materialized view's `PARTITION BY` among them. Add `MAP` to `Ident::can_be_printed_bare`'s quoting set, next to `LIST`, which is there for the analogous `list[1]` re-lex. The change is display-only: nothing that parsed before stops parsing, identifiers named `map` just print quoted. The comment claiming `MAP` was safe because `MAP[...]` requires `=>` only considered expression position, and is corrected. Found by the cargo-fuzz `sql_roundtrip` target. Adds a regression test beside the `LIST` one. The keyword-quoting fixtures in `tests/testdata/select`, `pretty.slt` and `quote_ident.slt` now show `"map"`, which is what `quote_ident` already returned for `list` and `array`. Co-Authored-By: Claude Opus 5 (1M context) --- src/sql-parser/src/ast/defs/name.rs | 18 +++++++++++++----- src/sql-parser/tests/sqlparser_common.rs | 22 ++++++++++++++++++++++ src/sql-parser/tests/testdata/select | 2 +- test/sqllogictest/pretty.slt | 2 +- test/sqllogictest/quote_ident.slt | 2 +- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/sql-parser/src/ast/defs/name.rs b/src/sql-parser/src/ast/defs/name.rs index f732b52ff8a06..9e4100637dc7e 100644 --- a/src/sql-parser/src/ast/defs/name.rs +++ b/src/sql-parser/src/ast/defs/name.rs @@ -19,7 +19,9 @@ // limitations under the License. use mz_ore::str::StrExt; -use mz_sql_lexer::keywords::{ALL, ANY, AS, DISTINCT, INTO, Keyword, LIST, PREPARE, SOME, WHEN}; +use mz_sql_lexer::keywords::{ + ALL, ANY, AS, DISTINCT, INTO, Keyword, LIST, MAP, PREPARE, SOME, WHEN, +}; use mz_sql_lexer::lexer::{IdentString, MAX_IDENTIFIER_LENGTH}; use serde::{Deserialize, Serialize}; use std::fmt; @@ -332,11 +334,17 @@ impl Ident { // `LIST` followed by `[` re-lexes as a `LIST[...]` literal // (`list[1]` is a valid one-element list), so a bare `list` // identifier that gets subscripted — `"list"[1]` — would - // reparse as a list literal instead of a subscript. (`ARRAY` - // is reserved-in-scalar-expression and so already quoted; - // `MAP[...]` requires `=>`, so `map[1]` is unambiguously a - // subscript.) + // reparse as a list literal instead of a subscript. + // (`ARRAY` is reserved-in-scalar-expression and so already + // quoted.) || kw == LIST + // An option value may be a `MAP[k => v]` literal, and that + // grammar commits to the map form on the `MAP` keyword + // alone, then demands `[`. So a bare `map` option value — + // `CREATE SINK … (TOPIC = map)` — fails to reparse rather + // than staying an identifier. Unlike expression position, + // there is no next-token lookahead to fall back on. + || kw == MAP // `DEALLOCATE [PREPARE] ` accepts an optional // `PREPARE` keyword before the name, so a bare `prepare` // name is consumed as that keyword on reparse, leaving no diff --git a/src/sql-parser/tests/sqlparser_common.rs b/src/sql-parser/tests/sqlparser_common.rs index 1e67fb0c313fc..fabeb64bfd8a5 100644 --- a/src/sql-parser/tests/sqlparser_common.rs +++ b/src/sql-parser/tests/sqlparser_common.rs @@ -878,6 +878,28 @@ fn test_list_keyword_bare_identifier_subscript_display_roundtrip() { } } +#[mz_ore::test] +#[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` +fn test_map_keyword_bare_identifier_option_value_display_roundtrip() { + // An option value may be a `MAP[k => v]` literal, and that grammar commits to + // the map form on the `MAP` keyword alone and then demands `[` — with no + // next-token lookahead to fall back on, unlike expression position. So a bare + // `map` option value fails to reparse ("Expected left square bracket") and + // `can_be_printed_bare` must quote it. Regression for the sql_roundtrip fuzz + // finding `CREATE SINK … (TOPIC = "map")`. + for sql in [ + r#"CREATE SINK s FROM t INTO KAFKA CONNECTION c (TOPIC = "map") FORMAT BYTES ENVELOPE DEBEZIUM"#, + r#"CREATE SOURCE s FROM KAFKA CONNECTION c (TOPIC = "map") FORMAT BYTES"#, + r#"CREATE MATERIALIZED VIEW v WITH (PARTITION BY = "map") AS SELECT 1"#, + r#"SELECT "map""#, + r#"SELECT "map"[1]"#, + // The map literal itself still prints as the special form. + r#"SELECT map['a' => 1]"#, + ] { + assert_display_roundtrips(sql); + } +} + #[mz_ore::test] #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` fn test_table_function_special_name_display_roundtrip() { diff --git a/src/sql-parser/tests/testdata/select b/src/sql-parser/tests/testdata/select index 57a834a31760d..0a685bb60bcfa 100644 --- a/src/sql-parser/tests/testdata/select +++ b/src/sql-parser/tests/testdata/select @@ -2102,6 +2102,6 @@ parse-statement SELECT "true", "false", "null", "array", "list", "map", "case", "cast", "coalesce", "greatest", "least", "nullif", "exists", "extract", "not", "row", "trim", "position", "substring" FROM iffy_colnames ---- -SELECT "true", "false", "null", "array", "list", map, "case", "cast", coalesce, greatest, least, nullif, exists, extract, "not", row, trim, position, substring FROM iffy_colnames +SELECT "true", "false", "null", "array", "list", "map", "case", "cast", coalesce, greatest, least, nullif, exists, extract, "not", row, trim, position, substring FROM iffy_colnames => Select(SelectStatement { query: Query { ctes: Simple([]), body: Select(Select { distinct: None, projection: [Expr { expr: Identifier([Ident("true")]), alias: None }, Expr { expr: Identifier([Ident("false")]), alias: None }, Expr { expr: Identifier([Ident("null")]), alias: None }, Expr { expr: Identifier([Ident("array")]), alias: None }, Expr { expr: Identifier([Ident("list")]), alias: None }, Expr { expr: Identifier([Ident("map")]), alias: None }, Expr { expr: Identifier([Ident("case")]), alias: None }, Expr { expr: Identifier([Ident("cast")]), alias: None }, Expr { expr: Identifier([Ident("coalesce")]), alias: None }, Expr { expr: Identifier([Ident("greatest")]), alias: None }, Expr { expr: Identifier([Ident("least")]), alias: None }, Expr { expr: Identifier([Ident("nullif")]), alias: None }, Expr { expr: Identifier([Ident("exists")]), alias: None }, Expr { expr: Identifier([Ident("extract")]), alias: None }, Expr { expr: Identifier([Ident("not")]), alias: None }, Expr { expr: Identifier([Ident("row")]), alias: None }, Expr { expr: Identifier([Ident("trim")]), alias: None }, Expr { expr: Identifier([Ident("position")]), alias: None }, Expr { expr: Identifier([Ident("substring")]), alias: None }], from: [TableWithJoins { relation: Table { name: Name(UnresolvedItemName([Ident("iffy_colnames")])), alias: None }, joins: [] }], selection: None, group_by: [], having: None, qualify: None, options: [] }), order_by: [], limit: None, offset: None }, as_of: None }) diff --git a/test/sqllogictest/pretty.slt b/test/sqllogictest/pretty.slt index 9248c0f5faf9a..6191939b51271 100644 --- a/test/sqllogictest/pretty.slt +++ b/test/sqllogictest/pretty.slt @@ -128,7 +128,7 @@ SELECT "null", "array", "list", - map, + "map", "case", "cast", coalesce, diff --git a/test/sqllogictest/quote_ident.slt b/test/sqllogictest/quote_ident.slt index 26753dd1a52aa..29f8fc7c31936 100644 --- a/test/sqllogictest/quote_ident.slt +++ b/test/sqllogictest/quote_ident.slt @@ -364,7 +364,7 @@ SELECT quote_ident('true'), quote_ident('false'), quote_ident('null'), quote_ide "null" "array" "list" -map +"map" "case" "cast" coalesce