Skip to content

sql: quote a bare map identifier so option values round-trip - #37993

Open
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:pr-sql-map
Open

sql: quote a bare map identifier so option values round-trip#37993
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:pr-sql-map

Conversation

@def-

@def- def- commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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.

`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) <noreply@anthropic.com>
@def-
def- requested a review from a team as a code owner July 31, 2026 20:35
@def-
def- requested a review from ggevay July 31, 2026 21:30
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.

1 participant