[ajitpratap0]

Summary

Follow-ups from the post-merge audit of #866–#868, about literal lexing and operator meaning:

• Numeric literals per dialect. Each dialect gets its real rules:

  • PostgreSQL 16 1_000, 0x1F, 0o17, 0b101
  • SQLite and ClickHouse digit separators
  • Oracle 1d/1f BINARY_DOUBLE/FLOAT and storage sizes (RESIZE 100M, BLOCKSIZE 8K)
  • MySQL digit-leading names (1col, t.1col, rendered with backticks)
  • 0x hex as on main

  Trailing junk such as 1x is now an error where the dialect rejects it. Before, it silently became 1 AS x.

• PostgreSQL Unicode escapes: U&'d\0061t' and U&"d\0061t" with UESCAPE, with comments allowed before UESCAPE. Before, these printed as U & '…'. Identifiers are written back decoded.

• Quoted type names keep their quotes, e.g. CAST(x AS "MyType") and PostgreSQL x::"char", through SQL(), FormatAST and Transpile.

• Precedence: PostgreSQL JSON operators now bind looser than ^, as PostgreSQL does.

• Operator meaning across dialects: formatting or transpiling into another dialect rewrites operators to the target's form or returns an error. It never emits text whose meaning changes. Covered: ^ (power vs XOR), #, ~, <<, >>, &, |, || (CONCAT in MySQL) and PostgreSQL-only operators. For example, PostgreSQL a # b used to reach MySQL as a # b, where # starts a comment. New FormatASTChecked / FormatStatementChecked / FormatExpressionChecked. Every in-repo caller uses them: gosqlx.FormatWithDialect, Transpile, transform, fingerprint, CLI, LSP, MCP and cbinding.

• SQL() on hand-built trees adds the parentheses they need, e.g. (a OR b) AND c, including a hand-built child inside a parsed tree. Parsed trees render byte-identically to main.

Test Plan

• gofmt, go vet, golangci-lint run (0 issues), task test:race, task test:cbinding; TestRealWorldCorpus green
• Parsed-tree SQL() over all testdata (251 files, 3,011 statements): identical to main apart from the intended changes
• Independent review and fix round. About 7,800 SQL strings × 9 dialects compared against main. The remaining differences are intended rejections (trailing junk, unsupported digit separators or bases, malformed escapes) and intended output changes. Truncation sweep plus 300k-input fuzz: no panics.
• Benchmarks: parser allocs/op unchanged. FormatWithDialect over the corpus is +2.5% ns/op and +1 alloc/op versus main.

Breaking Changes

Behaviour changes, listed per dialect in the CHANGELOG:

• Numeric forms with trailing letters (1x, 10M outside Oracle/generic) are errors where the dialect rejects them.
• Cross-dialect formatting of operators with no target equivalent returns an error from the Checked APIs, Transpile and FormatWithDialect. The legacy string-returning FormatAST/FormatStatement/FormatExpression return "" in that case, as their doc comments say.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

[vercel]

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

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 28, 2026 4:51am UTC

[github-actions]

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

---

Review of PR #874

I read the diff selectively, mainly pkg/sql/tokenizer/unicode_escape.go and pkg/formatter/dialect_scan.go. I did not run the build, vet or tests. The Bash call that would have run them needed approval and was denied, so I'm relying on the PR's stated test results.

Overall: The design looks sound. The operator-meaning checks fail loudly instead of emitting text whose meaning changes, and the *Checked API variants keep the legacy string-returning functions backward compatible. The test plan is thorough: corpus comparison against main, fuzzing, truncation sweeps and benchmarks.

Code quality and potential issues

1. unicode_escape.go:34-35: the error from skipSpaceAndComments() is dropped when it is non-nil, and the code falls into the "give back" branch. This looks safe, because the main loop should hit the same unterminated comment and report it. A short comment saying so would stop the next reader wondering whether the error is swallowed by mistake. It would also be worth a test with U&'x' /* unterminated.
2. unicode_escape.go:36-37: the UESCAPE keyword check uses strings.EqualFold on t.src but tests the word boundary through t.input. That is fine as long as src and input are always the same bytes. A one-line assertion or comment would make the invariant explicit.
3. unicode_escape.go:57: the UESCAPE character is rejected if it is a hex digit, +, a quote or whitespace. This matches PostgreSQL. Control characters other than whitespace and multi-byte runes are accepted. That is probably fine, but I'd confirm it is intended.
4. unicode_escape.go:200: value == 0 is rejected, which matches PostgreSQL's refusal of \0000. Good.
5. dialect_scan.go:10-125: this is a hand-written fast path that duplicates Children() for about 15 node types. Any new field added to one of these nodes (SelectStatement, FunctionCall, JoinClause and so on) must be added here too, or the operator check will miss it. A node added to a type's Children() but not to this scan would silently skip the cross-dialect check, so the output could change meaning without an error. Please add a test that walks the fixture corpus, runs Children()-based and fast-path scans on each, and asserts they agree. dialect_prepare_test.go may already cover part of this.
6. dialect_scan.go:49: SelectStatement only takes the fast path when Extras == nil && ArrayJoin == nil. Otherwise it falls through to Children(). This is correct, but it is a subtle perf cliff for ClickHouse-heavy workloads.
7. dialect_scan.go:46: this dereferences x.Extras only after a nil check, which is good. Line 22 calls x.DialectExtras().Escape unconditionally. Please confirm DialectExtras() can't return a nil pointer.

Performance

• The reported +2.5% ns/op and +1 alloc/op on FormatWithDialect is acceptable for the safety gained.
• Because the scan is a pre-pass over the whole tree, same-dialect formatting should skip it entirely. Please confirm a same-dialect early exit exists, since that is the common case.

Security

• Unicode-escape decoding validates UTF-8, surrogate pairs and hex digits, and bounds-checks every read. I found no panic path in decodeUnicodeEscapes.
• Decoding U&"..." identifiers into plain identifiers is the right choice. Check that the formatter re-quotes decoded names containing " or control characters.

Tests

• Coverage is broad and includes golden files for the tokenizer.
• Suggested additions: the unterminated-comment-after-literal case above, and a UESCAPE with a multi-byte escape character in both string and identifier forms.
• The breaking behaviour changes (1x becoming an error) are documented in the CHANGELOG, which is the right place for them.

Summary: No blocking issues found. The main risk is the duplicated traversal in dialect_scan.go drifting from Children(), so I'd add a parity test before merging.