[ajitpratap0]

Summary

VariantPath had no SQL() method, so expression rendering fell back to TokenLiteral() and emitted a bare : (WHERE src:a = 1 became WHERE : = 1). The formatter dispatches through SQL(), so it was broken too.

Adds VariantPath.SQL()/writeSQL (new file pkg/sql/ast/sql_variant_path.go): root, :field, .field, [index]. Field names are bare only if they pass the shared isBareNameChars rule (#632 helpers); otherwise they are double-quoted with embedded quotes doubled, so a hostile key cannot break out. Reserved words stay bare because the parser accepts keywords as path fields. ::type casts render through the existing CastExpression path (CAST(src:a AS int)). The parser is untouched.

Test Plan

• New pkg/formatter/render_variant_path_782_test.go: round-trip stability (parse, SQL(), parse, SQL()) for 17 forms (SELECT list, WHERE both sides, ORDER BY, GROUP BY, casts, nested/index/bracket/quoted/keyword fields); FormatAST compact and readable output re-parses and is stable; hostile-key test (quotes, semicolons, dots, colons, empty, digit-led).
• go build, go vet, gofmt clean; task test:race all green; TestPerformanceRegression passes.
• Parser benchmarks not run: no parser or tokenizer code changed.

Breaking Changes

None. Output that was previously a bare : is now valid SQL.

Closes #782

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 18, 2026 1:18pm UTC

[github-actions]

Claude finished @ajitpratap0's task in 16s —— View job

---

Review of PR #786: render Snowflake VariantPath in SQL() and the formatter

I read pkg/sql/ast/sql_variant_path.go and did not run the tests. I did not read the two test files in this pass. The fix looks correct and safe to merge, with a few small suggestions.

What looks good

• The fix is minimal and targeted. VariantPath.SQL() and writeSQL are in a new file, and the parser is untouched.
• Quoting is conservative. Names that are not bare-safe are double-quoted with embedded quotes doubled (via writeQuotedIdentifier), so a hostile key cannot break out of the path. This covers the main security concern.
• The -src:a case is handled correctly. The code comment explains why UnaryExpression roots are not parenthesized: wrapping would turn -(src:a) into (-src):a, which means something else.
• Operator-like roots (BinaryExpression, CastExpression, CaseExpression, BetweenExpression, InExpression) are parenthesized, so the path binds to the whole expression.
• The getBuf/putBuf pooling follows the repo's conventions, and nil receivers are handled.

Suggestions (non-blocking)

1. variantQuotedWords can drift. The comment says a brute-force test over the keyword tables guards it, and I did not check that test. If the tokenizer or keyword tables gain new token types, the list silently goes stale. Confirm the guard test fails loudly when that happens.
2. Other roots may need parentheses. The root switch is an allow-list of node types. Other low-precedence roots, such as IS NULL/LIKE-style expression nodes, would fall through to the default case and render unparenthesized. If those are not parseable as roots today, this is fine. A comment or a test pinning that assumption would help.
3. A segment with both Index and Name set. In a hand-built path, Index wins and Name is dropped silently. The commit messages mention hardening hand-built paths, so this is probably intended, but it is worth a one-line comment.
4. Quoting and case. The comment says Snowflake path elements are case-sensitive whether or not they are quoted. Please double check that claim against the Snowflake docs. Bare identifiers in : paths are case-sensitive, but the claim should be accurate before it is documented in code.
5. Changelog. Per CLAUDE.md, add an [Unreleased] entry to CHANGELOG.md. The PR changes only three files, none of them the changelog.

Test coverage
Per the PR description, the new tests cover round-trip stability for 17 forms, FormatAST in compact and readable modes, and hostile keys (quotes, semicolons, dots, colons, empty, digit-led). A second test file covers reserved words. That is solid coverage for this change.

The race-free and performance checks are reported as green in the PR description. I did not rerun them.
· branch fix/snowflake-variantpath-sql-782

[github-actions]

Claude finished @ajitpratap0's task in 19s —— View job

---

PR review: VariantPath SQL() rendering

I only read the code. The Bash tool needed approval for compound commands, so I did not run go build, go vet or the tests.

Verdict: looks good. I found no blocking issues.

What I checked

• pkg/sql/ast/sql_variant_path.go is the only production change, and it is additive. It adds SQL() and writeSQL, and doesn't touch the parser or tokenizer. That fits the "no breaking changes" claim and the decision to skip parser benchmarks.
• SQL() handles a nil receiver, and writeSQL does too.
• SQL() takes its buffer from the pool with getBuf() and returns it with defer putBuf(buf), as CLAUDE.md requires.
• Hostile field names are handled safely. Any name that isn't bare-safe is double-quoted through writeQuotedIdentifier, which doubles embedded quotes. Empty names and digit-led names also go through the quoted path.
• Segments are written correctly. The first field gets : and later fields get .. Index segments get [expr] and don't consume the :. A path like src[0]:a therefore renders correctly.
• Wrapping is sensible. Binary, cast, CASE, BETWEEN and IN roots are parenthesized so the path binds to the whole expression. Unary roots are deliberately left unwrapped, and the comment explains why: wrapping would turn -(src:a) into (-src):a.

Suggestions (non-blocking)

1. variantQuotedWords is a hand-maintained list. It will drift when someone adds a keyword token. The PR says a brute-force test over every keyword table guards it. I could see that test in the file list but did not read it, so please confirm it fails when a new keyword token is added.
2. Redundant ANY/SOME checks. The strings.EqualFold calls for ANY and SOME could go into variantQuotedWords so there is a single source of truth. A short note on why each word is there would help too.
3. Over-quoting. Names containing $ or non-ASCII letters (valid in Snowflake identifiers) go through the quoted path. That is safe and round-trips, and it matches the case-sensitivity note in the comment. It is worth mentioning in the doc comment so nobody "fixes" it later.
4. Index expressions. seg.Index goes straight through writeExpr, so a hand-built path with a nil Index and an empty Name renders as :"". That is valid, and the tests cover the empty-name case. I'd add a test for a negative or expression index such as src:a[i+1] if one isn't there already.
5. Two test files for one issue. render_variant_path_782_test.go and render_variant_path_reserved_782_test.go could be merged into one. That is cosmetic.

Security: quoting prevents a hostile path key from injecting SQL, and the hostile-key test covers quotes, semicolons, dots and colons.

Test coverage: 17 round-trip forms, formatter compact and readable output, hostile keys, and reserved words. That is solid coverage for an additive rendering change.

Please confirm task test:race is green in CI before merging.
· branch fix/snowflake-variantpath-sql-782

[github-actions]

Claude finished @ajitpratap0's task in 15s —— View job

---

Review of PR #786: render Snowflake VariantPath in SQL()

I read pkg/sql/ast/sql_variant_path.go. I did not read the two test files, and I did not build or run any tests.

Overall: The fix is small and focused. The parser is untouched. Field names are quoted safely.

Strengths

• Quoting is sound. writeQuotedIdentifier doubles embedded quotes, and empty or digit-led keys are quoted, so a hostile key can't break out of the path.
• Operator-like roots (binary, cast, case, between, in) are parenthesized, so the path binds to the whole expression.
• The comment explains why UnaryExpression is not wrapped. -(src:a) and (-src):a mean different things in Snowflake.
• SQL() uses the pooled buffer through getBuf/putBuf, as the rest of the package does.
• The nil receivers are handled.

Suggestions (non-blocking)

1. Hand-maintained variantQuotedWords (sql_variant_path.go:94). It is a copy of tokenizer behavior and can drift when keywords are added. Your description says a brute-force test over every keyword table guards it, and I didn't check that test. If the test is in the PR, keep it in CI. If a keyword's token type changes later, the test is what will catch it.
2. Case-sensitivity of unquoted fields. The doc comment says Snowflake treats path elements as case-sensitive whether or not they are quoted. Quoting never changes meaning, so the always-quote fallback is safe. It might be worth a one-line test showing that src:Foo round-trips with its case preserved.
3. Pre-existing unary behavior. The parser attaches the path to the operand in -src:a. That is a known quirk and out of scope here. Consider a follow-up issue to track it, so the comment doesn't become permanent.
4. Interaction with :: casts. CastExpression as a root is wrapped, which gives (CAST(src:a AS int)):b. This is correct, but make sure a test covers a path applied to a cast result.

Performance and security: No concerns. There are no extra allocations beyond the pooled buffer. Injection is mitigated by the quoting rules above.

Test coverage: From the PR description, the tests cover a round-trip for 17 forms, FormatAST output in compact and readable modes, and hostile keys. That is good. A reserved-word test file is also included.

No blocking issues from my read. I'd approve once CI is green.
· branch fix/snowflake-variantpath-sql-782

[github-actions]

Claude finished @ajitpratap0's task in 14s —— View job

---

Review of PR #786: render Snowflake VariantPath in SQL() and the formatter

I read pkg/sql/ast/sql_variant_path.go only. I did not read the two test files, and I did not build or run any tests.

Verdict: No blocking issues. The change is additive and small, and the parser is untouched.

What looks good

• Quoting is conservative. Any name that isn't bare-safe goes through writeQuotedIdentifier, which doubles embedded quotes. Empty and digit-led names are quoted too, so a hostile key can't break out of the path.
• Segment rendering is correct. The first field gets :, later fields get ., and index segments render as [expr] without consuming the :.
• Binary, cast, CASE, BETWEEN and IN roots are parenthesized, so the path binds to the whole expression.
• The comment on leaving UnaryExpression unwrapped is accurate. Wrapping would turn -(src:a) into (-src):a.
• getBuf/putBuf pooling and nil receivers follow the package conventions.

Suggestions (non-blocking)

1. variantQuotedWords (sql_variant_path.go:94) is hand-maintained. The comment says a brute-force test over the keyword tables guards it. Please keep that test in CI so it fails loudly when a new keyword token appears.
2. ANY/SOME check (sql_variant_path.go:82) is separate from the map. Moving both into variantQuotedWords would give one source of truth.
3. Root allow-list (sql_variant_path.go:46). Other low-precedence roots, such as IS NULL or LIKE, would fall to default and render unparenthesized. If the parser can't produce them as roots, a short comment saying so would help. Otherwise, add them to the list.
4. Hand-built segments with both Index and Name set (sql_variant_path.go:57). Index wins and Name is dropped silently. A one-line comment would document that.
5. Case-sensitivity claim in the doc comment (sql_variant_path.go:78). Please check it against the Snowflake docs before keeping it. The always-quote fallback is safe either way. A test that src:Foo round-trips with its case preserved would pin the behavior.
6. Over-quoting. Names with $ or non-ASCII letters take the quoted path. That is safe, but a note in the doc comment would stop someone from "fixing" it later.
7. Extra test cases. Please make sure the tests cover an expression index such as src:a[i+1], and a path applied to a cast result such as (CAST(src:a AS int)):b.
8. Changelog. CLAUDE.md asks for an [Unreleased] entry in CHANGELOG.md. This PR doesn't change that file.

Performance and security: No concerns. There are no allocations beyond the pooled buffer, and the quoting rules prevent injection through path keys.

Test coverage: From the PR description, the tests cover round-trip stability for 17 forms, FormatAST in compact and readable modes, hostile keys, and reserved words. I'd approve once CI (task test:race) is green.
· branch fix/snowflake-variantpath-sql-782