[ajitpratap0]

Summary

#521 [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins

Fixes #521. Column references with more than two parts now parse in every dialect: db.table.col, db.t.* and catalog.schema.t.col. The extra qualifiers are folded into Identifier.Table as a dotted string, mirroring TableReference.Name, so SQL() reproduces the reference without any AST change. A parenthesized join group at the start of FROM, as in MySQL SHOW CREATE VIEW output, is now parsed recursively with a depth guard and its joins are flattened into the statement's join chain, so the AST and SQL() are identical to the flat query. Group shapes that flattening would get wrong (comma lists after or inside a group) are rejected with E4001. Verification: the new regression tests fail on the parent commit and pass on this branch. Differential testing of about 3,866 corpus statements in 9 dialects showed no changed output for any input that parsed before. In an alternating benchstat run, parser allocs/op and B/op are unchanged and ns/op is flat (geomean -0.86%). The race suite, task check and golangci-lint all pass.

Commits (1), one per issue:

• 18c73f2 fix(parser): parse multi-part column references and parenthesized join groups ([BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521)

5 files changed, 509 insertions(+), 12 deletions(-)

Test Plan

• Independent validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 120s $(go list ./... | grep -v /cbinding)), go vet, and golangci-lint on this branch.
• Each change was reviewed by an independent reviewer with access to the code and benchmarks before validation.
• Benchmarks compared with benchstat -count=6 before/after on the same machine; allocs/op and B/op are the deterministic signal, ns/op is directional on a loaded machine.

#521 benchstat

I built test binaries from a git-archive of the parent commit (142bff2) and from HEAD (18c73f2), then ran them alternately (6 rounds, count=1 each, Apple M4) and compared with benchstat. allocs/op and B/op are identical in every sample: SimpleSelect 9 / 1.656Ki, ComplexSelect 22 / 5.408Ki, Insert 16 / 2.039Ki, Update 13 / 1.625Ki, Delete 9 / 1.359Ki. ns/op shows no regression: SimpleSelect 407.8n→414.9n (~, p=0.093), ComplexSelect 1.381µ→1.379µ (~), Insert 516.5n→504.6n (~), Update 404.0n→396.3n (-1.91%, p=0.015), Delete 307.7n→302.8n (~), geomean -0.86%. The +34% to +65% ns/op in the implementer's report was machine noise.
Bug reproduction: I copied the new mysql_issue521_test.go and the updated qualified_identifier_test.go onto the parent archive. TestIssue521_ExactReport (mysql, mariadb), ParenthesizedJoinEquivalentToFlat (all dialects), RoundTrip, ExtractTables, Rejections (comma/nesting/trailing-dot) and the new QualifiedIdentifier_Shape rows all FAIL there and pass on HEAD.
CLI check: I built cmd/gosqlx from the worktree. `gosqlx validate --dialect mysql` on the verbatim issue query (taken from the issue body via gh) prints "Valid SQL". `gosqlx format` renders `sakila.city.city AS city` and a flat `FROM sakila.customer AS cu INNER JOIN sakila.address AS a ... INNER JOIN sakila.city ... INNER JOIN sakila.country ...`.
Differential: I extracted 3,837 SQL strings from the repo's tests and .sql files, added 29 hand-written edge cases, and ran all 3,866 through the parent and HEAD in 9 dialects (34,794 parses). SQL() and FormatAST(Compact) were compared. Result: 0 OK→OK output changes, 0 OK→ERR, 0 panics. There were 155 ERR→OK, all multi-part identifiers or parenthesized join groups, and 88 ERR→ERR where only the error message changed.

#521 race gate: PASS. CGO_ENABLED=1 go test -race -timeout 120s $(go list ./... | grep -v /cbinding) exited 0: 44 packages ok, no FAIL, no DATA RACE. Many results were cached, so I re-ran -race -count=1 on pkg/sql/parser (3.9s), pkg/gosqlx and pkg/formatter, and all were ok. task check (fmt, vet, lint, test:race) also exited 0, and git status --porcelain was still empty afterwards.

Breaking Changes

• [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521: None to exported signatures or AST types. ast.Identifier gets no new field; the alternative of adding a Schema field was rejected because it would break unkeyed literals and every consumer. Three or more qualifiers are stored in the existing Table field as a dotted string. That matches TableReference.Name ("db.table") and the existing Identifier.SQL() handling, since identifierNeedsQuoting allows '.'. The only signature change is to the unexported parser method parseJoinClauses, which gains a joins []ast.JoinClause argument (one caller). Observable behaviour: SQL that used to fail (multi-part column references; left-nested parenthesized joins in FROM) now parses in all dialects. A parenthesized join group renders flattened, and its AST is identical to the flat join chain. Some invalid inputs get more specific error messages, including new E4001 unsupported-feature errors for comma lists combined with parenthesized joins. Nothing that parsed before parses differently.

Notes

• CHANGELOG.md and performance_baselines.json are intentionally not touched on this branch; one consolidated follow-up PR will update both for all branches of the perf audit (tracking issue perf: repo-wide optimization audit (2026-10-11) — tracking issue #566).
• Base: main.

Fixes #521
Part of #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

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

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 13, 2026 8:18am UTC

[github-actions]

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

---

Review of PR #585: three-part identifiers and parenthesized join groups (#521)

I reviewed the diff by reading it. I did not build the branch or run the tests, so I'm relying on the test results reported in the PR description.

Verdict: looks good. I found nothing blocking.

What I checked

• Three-part identifiers (expressions_literal.go): The two-part path is unchanged, and the extra loop runs only when a third . follows. Folding the qualifiers into Identifier.Table as a dotted string matches TableReference.Name, so SQL() round-trips without an AST change. The db.t.* case correctly rejects a trailing . after *. Pulling isQualifiedNamePart() out as a helper removes duplicated logic.
• Parenthesized joins (select_clauses.go): Flattening is semantically correct because joins associate to the left, so ((a JOIN b) JOIN c) JOIN d is the same as the flat chain. The depth guard uses the existing MaxRecursionDepth with a deferred decrement, so it can't leak on error paths. Rejecting comma lists after or inside a group with E4001 is the right call, since hoisting the joins would silently change which table they bind to. isParenthesizedJoinStart peeks only at identifier, double-quoted string, or (, so (SELECT ...) derived tables keep their existing path and error messages.
• Performance: No new allocations on the hot path, which agrees with the benchstat numbers in the description. The unexported parseJoinClauses signature change has a single caller.

Minor suggestions (non-blocking)

1. Backtick-quoted first table. isParenthesizedJoinStart doesn't list a dedicated backtick-quoted token type. The MySQL test using `sakila`.`customer` passes, so these tokens presumably lex as identifiers. A short comment saying so would help, and so would a test with a quoted name directly after (, such as FROM (`a` JOIN `b` ON ...).
2. Alias after a group. FROM (a JOIN b ON ...) AS x is not valid MySQL for a joined table, but I'd add a negative test pinning the resulting error so it doesn't change by accident. The same goes for FROM (a) followed directly by a comma, which should still parse.
3. Dotted Table value. Consumers that read Identifier.Table as a single qualifier, such as the linter, LSP and ExtractTables, will now see "db.t". The PR notes this is intentional and the ExtractTables test covers it. A one-line doc comment on the Identifier.Table field would help future readers.
4. Changelog. CHANGELOG.md is deliberately left for a consolidated follow-up PR, per the description. Please make sure that follow-up covers [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521.

Test coverage: The new tests are thorough. They cover the exact issue query on MySQL and MariaDB, equivalence with the flat query across all dialects, round-trip, ExtractTables, and the rejection cases. The description also reports differential testing with no change to any input that parsed before.
| Branch: fix/mysql-three-part-ident-paren-joins

[github-actions]

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

---

Review of PR #585: three-part identifiers and parenthesized join groups (#521)

I reviewed the diff by reading it, including the newer hardening commit. I did not build the branch or run the tests, so I'm relying on the results reported in the PR description.

Verdict: looks good. I found nothing blocking, and one item below is worth checking before merge.

Addressed since the earlier review

• Identifier.Table now has a doc comment that explains the dotted form and the quoted-dot ambiguity (pkg/sql/ast/ast.go).
• An alias or ClickHouse FINAL after a group is rejected with E4001. Silently attaching either one to the first table would have been wrong.
• parseMultiPartColumnRef caps references at 5 parts, collects them in a fixed array, and joins them once. Cost stays linear and the two-part path is unchanged.
• ExtractColumnsQualified splits the qualifier at the last dot, so db.t.col agrees with ExtractTablesQualified.
• SQL() quotes each dotted part separately through writeDottedIdentifier and qualifierNeedsQuoting. Leading, trailing and doubled dots are handled.

Worth checking

1. Possible regression for (( in FROM. isParenthesizedJoinStart returns true for ( followed by (. A query like FROM ((SELECT 1) UNION (SELECT 2)) t, or any other parenthesized set operation used as a derived table, now goes through parseParenthesizedJoin. The inner (SELECT ...) parses as a derived table, but the next token is UNION rather than a join keyword or ). That would produce "expected ) or JOIN", where it previously took the derived-table path, if that path accepted this input. I couldn't run it here. The differential corpus may not contain this shape. Please run it against main and add a test either way. If it parsed before, one fix is to fall back (for example by backtracking or by peeking past the nested parens) when the group isn't followed by a join or ).
2. Test the quoted first table. Add FROM (`a` JOIN `b` ON ...) to confirm that backtick names lex as TokenTypeIdentifier or DoubleQuotedString after (. Also add one comment on the switch in isParenthesizedJoinStart saying so.

Minor notes (non-blocking)

• A dotted Table value is still a convention. Consumers that read Table as a single qualifier, such as the linter and LSP, will now see "db.t". Fine as documented, but the follow-up changelog should call it out.
• Make sure the consolidated changelog PR (tracking issue perf: repo-wide optimization audit (2026-10-11) — tracking issue #566) covers [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521.

Performance and security: No new allocations on the hot path. Recursion is bounded by MaxRecursionDepth with a deferred decrement, and the part count is capped. I saw no security concerns.

Test coverage: Thorough. It covers the exact issue query on MySQL and MariaDB, equivalence with the flat query in all dialects, round-trip, extraction, and the rejection cases. Item 1 above is the main gap.
| Branch: fix/mysql-three-part-ident-paren-joins

[ajitpratap0]

Pre-merge review follow-ups (merge train), in the latest commits:

• Quoted multi-part names now round-trip: my-db`.`t`.`c renders as "my-db".t.c instead of the single identifier "my-db.t".c. A quoted qualifier containing both a dot and an unsafe character is inherently ambiguous in the dotted Identifier.Table form; documented on the field.
• Column references are capped at 5 parts and joined once (the 1 MB dotted chain went from ~10 s to ~10 ms rejection).
• ExtractColumnsQualified splits db.t.col into Schema/Table/Name consistently with ExtractTablesQualified.
• An alias or ClickHouse FINAL after a parenthesized join group is rejected with E4001 instead of a confusing error (FINAL was previously applied to the first table only).
• Error snippets no longer print the hint text as source.
• Checked the FROM ((SELECT ...) UNION (SELECT ...)) t concern: those shapes fail on main too ("expected SELECT in derived table"); this PR newly parses ((SELECT ...) t JOIN y ...) and ((a JOIN b ...)), and only changes error text for the rest. Parenthesized set operations as derived tables are tracked as a follow-up.
• Rebased onto perf(tokenizer): Oracle bind variables, substring token values, zero-copy quoted literals, dialect pooling (#520, #546, #548, #550) #595 (shared files merged cleanly); race tests green.

[github-actions]

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

---

Review of PR #585: three-part identifiers and parenthesized join groups (#521)

I reviewed this by reading the code. I tried to run probe tests for the open items below, but the sandbox denied go test, so nothing here has been run. I'm relying on the test results reported in the PR description.

Verdict: looks good. I found nothing blocking. One item is still open from the last review.

Open item: (( in FROM

• isParenthesizedJoinStart (pkg/sql/parser/select_clauses.go:142) returns true for ( followed by (.
• A parenthesized set operation used as a derived table, such as FROM ((SELECT 1) UNION (SELECT 2)) t, would now go to parseParenthesizedJoin. The inner (SELECT ...) parses as a derived table, and the next token is UNION, not a join keyword or ).
• If main accepted that shape, this is a regression. The differential corpus may not contain it.
• Please run this on main and on the branch, and add a test either way:
  • SELECT * FROM ((SELECT 1) UNION (SELECT 2)) t
  • If it parsed before, fall back to the derived-table path when the group isn't followed by a join or ).
  • If it never parsed, pin the current error message in a test.

Still to add

• A test for a backtick-quoted first table directly after (, such as FROM (`a` JOIN `b` ON ...). The switch in isParenthesizedJoinStart lists only TokenTypeIdentifier, TokenTypeDoubleQuotedString and TokenTypeLParen. A one-line comment saying which type backtick names lex as would help.

Addressed since the earlier review

• Identifier.Table now has a doc comment.
• An alias or FINAL after a group is rejected with E4001.
• Multi-part references are capped at 5 parts.
• ExtractColumnsQualified and SQL() handle dotted qualifiers consistently.

Notes

• A dotted Table value is a convention. Consumers that read it as a single qualifier, such as the linter and LSP, will see "db.t". The follow-up changelog should mention this.
• The consolidated changelog PR (perf: repo-wide optimization audit (2026-10-11) — tracking issue #566) should cover [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521.
• Recursion is bounded by MaxRecursionDepth with a deferred decrement. I saw no security concerns, and no new hot-path allocations.
• Test coverage is thorough: the exact issue query on MySQL and MariaDB, flat-query equivalence, round-trip, extraction, and the rejection cases. The (( shape is the main gap.
  | Branch: fix/mysql-three-part-ident-paren-joins

[github-actions]

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

---

Review of PR #585: three-part identifiers and parenthesized join groups (#521)

I reviewed this by reading the code. I did not build the branch or run the tests, so I'm relying on the results in the PR description.

Verdict: looks good. I found nothing blocking.

Both open items from the earlier reviews are resolved

• (( in FROM: Your 2026-10-13 comment says FROM ((SELECT ...) UNION (SELECT ...)) t already fails on main with "expected SELECT in derived table", so this isn't a regression. I couldn't confirm that by running it. Two small suggestions:
  • Pin the current error for that shape in a test, so a later follow-up that adds parenthesized set operations as derived tables changes it on purpose.
  • Link the follow-up issue from the PR.
• Backtick-quoted first table: This is now covered. pkg/sql/parser/mysql_issue521_review_test.go:213 tests FROM (`a` JOIN `b` ON ...) JOIN `c` .... That is enough evidence that backtick names lex as TokenTypeIdentifier or TokenTypeDoubleQuotedString.

Optional

• Add a one-line comment on the switch in isParenthesizedJoinStart (select_clauses.go:146) saying that backtick names arrive as one of those two token types. It's cheap and saves the next reader the same question.

Notes for the consolidated changelog PR (#566)

• Cover [BUG] Valid MySQL query fails on three-part identifiers and parenthesized joins #521.
• Say that Identifier.Table can now hold a dotted qualifier such as "db.t". Consumers that read it as a single qualifier, such as the linter and LSP, will see that form.

Performance and security

• Recursion is bounded by MaxRecursionDepth with a deferred decrement.
• Multi-part references are capped at 5 parts, so the long-dotted-chain rejection stays cheap.
• I saw no new hot-path allocations and no security concerns.

Test coverage: Thorough. It covers the exact issue query on MySQL and MariaDB, flat-query equivalence, round-trip, extraction, quoted names, and the rejection cases.
| Branch: fix/mysql-three-part-ident-paren-joins