sql-parser: quote bracketed ids that need it on display

[bosconi]

Quote bracketed ids that need it on display so the printed SQL parses back.

• Cause: display printed a keyword id like row bare in [<id> AS <name>], and printed [<id>] cluster and network policy ids unquoted.
• Cause: the parser accepts only an identifier token for these ids (or a number for [<id>]), so the output failed with "expected id".
• Fix: one shared helper prints the id bare only if it lexes back as the same token, and quotes it otherwise.
• Real ids like u1 and numeric cluster ids like [1] still print bare, so there is no user-facing change.
• Tests: five regression cases in src/sql-parser/tests/testdata/id, covering item names, casts, clusters, network policies and the fuzz finding.

Closes: SQL-769

🤖 Generated with Claude Code

[linear-code]

SQL-769

[def-]

QA LLM Review

1. MEDIUM -- Long numeric [<id>] cluster/network-policy ids now panic on display or print unparseable SQL

src/sql-parser/src/ast/metadata.rs:120

write_bracketed_id always builds Ident::new_unchecked(id), but parse_raw_ident_str accepts a Token::Number of any length. Any IN CLUSTER [<256+ digits>] or NETWORK POLICY [<256+ digits>] therefore trips the len <= MAX_LENGTH soft assert while being displayed, which aborts environmentd wherever soft assertions are on (CI, mzcompose, testdrive, slt, bin/environmentd, cargo-fuzz). A long non-integer number such as [1.<300 digits>] gets quoted into a >255-byte identifier that the lexer rejects, so the round trip this PR is meant to guarantee breaks. The parent commit handles both inputs: it displays them without panicking and they reparse.

Details

The parser fills these ids from Some(Token::Number(n)) => n (src/sql-parser/src/parser.rs:4487). lex_number puts no limit on the length and keeps ./E in the string. Only Token::Ident is capped at 255 bytes.

Probe results at the PR head:

• CREATE INDEX i IN CLUSTER [<300 ones>] ON t (a) with soft assertions on: panic at src/sql-parser/src/ast/defs/name.rs:134 (assertion failed: s.len() <= Self::MAX_LENGTH). The same happens for COMMENT ON NETWORK POLICY [...].
• With soft assertions off, IN CLUSTER [1.<300 ones>] displays as ["1.111…"] and reparsing fails with identifier length exceeds 255 bytes. NETWORK POLICY [<300 ones>.5] fails the same way.

At the parent commit, all three cases display without panicking and reparse.

In environmentd, every statement goes through Session::mint_logging → PreparedStatementLoggingInfo::still_to_log, which calls to_ast_string_redacted() before name resolution. So a single statement from any SQL user is enough. Production builds without MZ_SOFT_ASSERTIONS do not panic, and the unparseable output only occurs for ids that never resolve.

Suggested fix: make the bare-or-quote decision match the doc comment ("lexes back as the same token") and don't construct an Ident. Print the id bare when lexing it yields exactly [Token::Ident(id)], or [Token::Number(id)] when allow_number is set. Otherwise quote it by hand, writing " + the id with " doubled + ". Number ids then always print bare whatever their length or shape, and quoting only applies to ids that came from a length-capped identifier token.

[ggevay]

One simplification inline, which also fixes the QA bot's finding.

(The bug itself is fuzzer-only: real ids always look something like u1, which are not keywords, so prod can't hit it.)

[ggevay]

Simpler: drop Token::Number from parse_raw_ident_str instead of handling it here. It's left over from integer cluster ids (IN CLUSTER [1], 395050f). ClusterId::from_str now needs an s/u prefix, so a numeric cluster id never resolves, and only three testdata/ddl cases use one. (Network policies resolve the bracket text as a name, and ["1"] still covers that.) Then this becomes:

fn write_bracketed_id<W: fmt::Write>(f: &mut AstFormatter<W>, id: &str) {
    let ident = Ident::new_unchecked(id);
    if ident.has_only_bare_chars() && ident.as_keyword().is_none() {
        f.write_str(id);
    } else {
        f.write_str(ident.to_ast_string_stable());
    }
}

That also fixes the bot's finding: new_unchecked runs before the number check, so IN CLUSTER [<300 digits>] hits the 255-byte soft assert. Identifier tokens are capped at 255 bytes.

Also, the comment on the empty-id check in parse_raw_ident_str is stale with this change ([""] would now print as [""]). Please keep the check, but give the reason that parse_identifier rejects zero-length identifiers everywhere else, and let parse_raw_name reuse parse_raw_ident_str so item ids get it too.

[ggevay]

Please add it to the comment that this is just a fuzzer-only requirement, real queries can't hit it.