[ajitpratap0]

Summary

#613 parser: parseQualifiedName concatenates dotted table names quadratically

parseQualifiedName (pkg/sql/parser/parser.go) used to build dotted object names with repeated string concatenation and no part cap, so cost grew quadratically: a 64 KB a.a.a... chain in FROM or DROP TABLE took about 66 ms and 1.17 GB and parsed successfully. It now collects up to 5 parts in a stack array (verified not to escape) and joins them once. Names with more than 5 parts are rejected with E2004 'qualified name has more than 5 parts', the same limit #585 set for column references. The same 64 KB input now fails in about 8 ms with 10.5 MB. Names with 1 to 5 parts render byte-identically on every caller path (FROM, JOIN, DML, CREATE/DROP/TRUNCATE, views, indexes) compared with main 61ca6c8. Benchmarks: allocs and B/op unchanged on Parser{Simple,Complex}Select/Insert/Update/Delete; 3-part and 5-part names drop 1 and 3 allocs; chain4KB drops 98.6% of allocs and 86.8% of B/op. Known limitations and deferred items: only the FROM path surfaces the E2004. JOIN, DML and DDL callers keep their existing E2002 'expected table name' wrapping and were left untouched to avoid conflicts with the from-joins and insert chains. The Snowflake permissive CREATE path now keeps only the trailing part of an over-long name in RawSQL. The change was verified by new regression tests that fail on main, the full -race suite, go vet, golangci-lint, and interleaved benchstat runs.

#616 parser: several E2002 errors pass hint text as the SQL source, printing a bogus context snippet

Fixes #616. Fifteen parser error sites passed an explanatory sentence as the builder's sql argument: 9 ExpectedTokenError (E2002) sites in FROM, SAMPLE x5, PREWHERE, WHERE and the SELECT column list, plus 6 InvalidSyntaxError (E2004) sites in CAST/::TYPE params, INTERVAL, the empty array subscript and both strict-mode empty-statement checks. Error() then printed that sentence as line 1 of the user's SQL, with a caret at the real column. Each site now passes "" and attaches the same sentence through .WithHint(...), so the code, message and line/column are unchanged; only the fake snippet is gone and the sentence now appears as the Hint. Verified with a table-driven regression test covering every scout reproduction, a go/ast guard that fails if any non-test parser file passes a non-empty literal as a pkg/errors builder's sql argument, and a byte-for-byte comparison of the error headers before and after. The race suite (excluding cbinding) passes. Out of scope, with follow-ups: showing the real SQL in parser error context, which means threading source text into Parser or attaching it in gosqlx/CLI/LSP and would change output at roughly 100 sites; ClickHouse parsing FROM t SAMPLE / SAMPLE x with SAMPLE as a table alias; polishing the wording of the hints.

Commits (2), one implementation commit per issue plus test-only follow-ups:

• f5d80b8 fix(parser): stop passing hint text as the SQL source in E2002/E2004 errors (parser: several E2002 errors pass hint text as the SQL source, printing a bogus context snippet #616)
• 05eee99 perf(parser): join qualified object names once and cap them at 5 parts (parser: parseQualifiedName concatenates dotted table names quadratically #613)

8 files changed, 499 insertions(+), 35 deletions(-)

Test Plan

• Each issue's reproduction cases were run against main before the fix (Haiku scout), and a failing regression test was written before the code change.
• Independent review (Opus, or Sonnet on small chains) with a bounded fix loop.
• Independent Opus validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding)), go vet, golangci-lint, re-ran the reproduction cases on HEAD, and confirmed the new tests fail on the parent commit.
• Perf items: benchstat -count=6 before/after on the same machine.

#613 evidence (validator)

I ran the benchmarks myself with the parent built from git archive 61ca6c8, -count=6 and -benchtime=200ms (files val/before.txt and val/after.txt).
- allocs/op: identical for ParserSimpleSelect (7), ComplexSelect (20), Insert (14), Update (11), Delete (7) and Issue613/1part (8). 3part went from 10 to 9 (-10%), 5part from 12 to 9 (-25%), and chain4KB from 2068 to 29 (-98.60%).
- B/op: unchanged for the Parser* benchmarks and 1part. 3part and 5part dropped by 0.7%. chain4KB went from 5032.8Ki to 663.5Ki (-86.82%).
- ns/op: chain4KB went from 892.7µs to 171.9µs (-80.75%). In the sequential run, the Parser* benchmarks showed +27% to +139% ns/op with unchanged allocs. That run was taken under load average 8 to 16 and is not reliable.
- I re-ran Parser* interleaved: 8 alternating rounds of prebuilt before and after test binaries, benchtime=300ms (val/b2.txt, val/a2.txt). Every result was '~' (p=0.57 to 1.0), with a geomean of -8.19%. There is no real ns/op regression.

Scout re-run on HEAD versus the parent, using a gosqlx.Parse program in val/scout:
- FROM chains: 16KB went from 18.7ms/73.6MB to 0.9ms/2.7MB, 32KB from 28.7ms/288MB to 1.6ms/5.3MB, and 64KB from 68ms/1166MB to 8ms/10.5MB. On HEAD each one returns E2004 'qualified name has more than 5 parts'. 128KB on HEAD takes 15ms/21MB and returns E2004.
- DROP TABLE with a 64KB chain went from 66ms/1166MB with err=nil to 8ms/10.6MB with E2002 'expected object name'.
- Column refs are unchanged: E2004 for 64KB and for 6 parts.
- SELECT * FROM a.b.c.d.e.f went from err=nil to E2004.
- Across all 13 templates and 7 name shapes (t, s.t, d.s.t, srv.d.s.t, a.b.c.d.e, a.b.c.d.e.f, a.), the HEAD and parent outputs differ only on the 6-part rows. 1 to 5 part names and trailing-dot errors render byte-identically.

Regression tests against the parent (test file copied into the git-archived 61ca6c8 tree): TestIssue613_QualifiedNamePartLimit and TestIssue613_LongQualifiedNameFailsFast FAIL. The accepted, four-part and trailing-dot t

#613 race gate: PASS. CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) on HEAD 05eee99 printed EXIT=0 with 44 packages ok and no FAIL lines. No re-runs were needed. Log: /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/613/val/race.txt
#616 evidence (validator)

Not a perf item, so benchstat does not apply. Repro evidence: I ran all 15 scout cases through gosqlx.Parse/ParseWithDialect on a git-archive snapshot of HEAD and of parent 05eee99, both under scratchpad/b3/616/{head,parent}. On the parent, Error() prints the hint sentence as a fake source line, e.g. "   1 | WHERE clause requires a boolean expression" with carets under it, and the hint reads "expected expression after WHERE here". On HEAD no source line is printed and the hint is the sentence verbatim ("Hint: WHERE clause requires a boolean expression"). The header lines ("Error E2002 at line L, column C: expected X, got Y") are byte-identical between parent and HEAD for all 15 cases. The JOIN control case matches on both. The CLI command `go run ./cmd/gosqlx validate "SELECT a FROM t WHERE ) "` on HEAD shows the header, then the hint, and no fake source line. With error_context_issue616_test.go copied into the parent snapshot, all 16 table subtests fail, plus both strict-mode subtests and the static guard (15 offenders). Every one of those failures is a Context.SQL or hint mismatch. None is a code, message or location mismatch, which confirms those fields did not change. All three tests pass on HEAD.

#616 race gate: CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) on HEAD f5d80b8 exited 0. 44 packages reported ok, with 0 FAIL lines and 0 DATA RACE reports. A few packages, such as tokenizer, were served from the test cache, which is keyed on their content. The full log is at /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/616-race.txt. Nothing flaked, so no re-run was needed.

Breaking Changes

• parser: parseQualifiedName concatenates dotted table names quadratically #613: The exported API is unchanged; parseQualifiedName and the new const maxQualifiedNameParts are unexported. Observable behaviour changes in one way: dotted object names with more than 5 parts in FROM, DML targets and DDL names (TABLE, VIEW, INDEX, USE, MySQL statements) are now rejected with E2004 'invalid syntax: qualified name has more than 5 parts', where they previously parsed. This is intentional, follows the issue, and mirrors the existing E2004 limit for column references from fix(parser): support three-part identifiers and parenthesized join groups (#521) #585. Names with 1 to 5 parts produce exactly the same AST strings and rendered SQL as before.
• parser: several E2002 errors pass hint text as the SQL source, printing a bogus context snippet #616: No exported API or signature changes. Observable behaviour changes only for the 15 listed error sites. Error.Context.SQL is now "" instead of the explanatory sentence, so Error() no longer prints a fake "line 1" with a caret. Error.Hint is now that explanatory sentence instead of the generic auto-generated hint. Code, Message and Location are unchanged. This is additive and corrective, and consistent with fix(parser): support three-part identifiers and parenthesized join groups (#521) #585.

Notes

• CHANGELOG.md is intentionally not touched; the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 gets the entries for this batch before it merges.
• Part of the batch-3 merge train (follow-ups cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599-schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620); later PRs in the train are rebased onto this one after it merges.
• Base: main.

Fixes #613
Fixes #616
Follow-up to #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 14, 2026 8:40am UTC

[github-actions]

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

---

Review of PR #627

Overall this looks good. The changes are small and targeted, and the tests are well considered. I read the diff of parser.go and select.go and did not run the test suite.

#613 parseQualifiedName

• Collecting parts in a [5]string array and calling strings.Join once is a correct linear-time fix. Rejecting a 6th part early also bounds the work on hostile input.
• The comment above maxQualifiedNameParts is inconsistent. It says "Four parts cover the longest form any supported dialect accepts", but maxColumnRefParts is 5 (expressions_literal.go:487). Either update the comment or explain why 5 is allowed, for example by noting that column refs add a trailing part. It is worth fixing so the next reader doesn't "correct" the constant.
• The limit check runs after the . is consumed and before the next token is validated. So a.b.c.d.e. (trailing dot after 5 parts) reports "more than 5 parts" instead of "expected identifier after .". This is minor, but moving the check after the identifier validation would give the more accurate error. Check whether the PR's trailing-dot tests cover only shorter names.
• The E2004 error is only surfaced from the FROM path. JOIN, DML and DDL callers wrap it as E2002 "expected table name", as the PR notes. That is acceptable as a documented follow-up. Please file an issue so it isn't lost.
• The Snowflake CREATE <kind> path now keeps only the trailing part in RawSQL for over-long names. This is documented in the PR and seems harmless, but it is a silent behaviour change, so a test pinning it would help.
• There is a behaviour change: 6+ part names that used to parse are now rejected. This is intentional and called out in the PR description, so a CHANGELOG entry via docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 is the right place for it.

#616 hint text vs SQL source

• Passing "" and attaching the sentence with .WithHint(...) is the right fix. The code, message and location stay the same, and the fake snippet disappears.
• The go/ast guard test that fails on non-empty literal sql arguments is a nice way to prevent regressions. Check that it doesn't skip calls built through helper wrappers or variables, since those would evade a literal-only check.

Performance and security

• The stack array avoids heap allocation, and the benchmarks in the PR show fewer allocations for 3-part and 5-part names. The cap removes a quadratic-memory DoS vector (1.17 GB for a 64 KB input).
• I have no security concerns.

Tests

• Coverage is good: table-driven reproduction cases, a limit test, and a fail-fast test. I'd add a test for the trailing dot after 5 parts if the error-ordering point above is addressed.

No blocking issues. The only change I'd ask for is the comment fix.

[github-actions]

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

---

Review of PR #627 (re-review after the latest push)

I read the diff of parser.go, ddl.go and select_subquery.go. I did not run the test suite.

The head has moved on since the earlier review. JOIN and DDL callers now pass the E2004 through via isQualifiedNameTooLong and qualifiedNameError, so the "only FROM surfaces E2004" limitation in the PR description looks outdated. Please update the description.

Remaining nits

• Stale comment (parser.go, above maxQualifiedNameParts): it says "Four parts cover the longest form any supported dialect accepts", but the constant is maxColumnRefParts, which is 5. Reword it to say the limit is shared with column refs on purpose, so nobody "fixes" it to 4.
• Trailing-dot error order: the limit check runs right after consuming the ., before the next token is validated. a.b.c.d.e. therefore reports "more than 5 parts" instead of "expected identifier after .". Checking after the identifier test would give the accurate message. If the current behavior is intended, add a test that pins it.
• Error-comparison helpers: isQualifiedNameTooLong relies on WithCause plus errors.Is. This is fine as long as pkg/errors implements Unwrap. Please confirm the new tests cover each caller path (FROM, JOIN, DML, DDL, USE), not just FROM.
• Snowflake permissive CREATE: it now returns the E2004 for an over-long name rather than keeping a truncated tail in RawSQL. This is a better outcome than what the PR description says, but the description should reflect it.
• parser: several E2002 errors pass hint text as the SQL source, printing a bogus context snippet #616 guard test: a go/ast check on literal sql arguments won't catch values passed through variables or wrapper helpers. That is acceptable, but say so in the test's comment.

Performance and security

• The [5]string stack array with a single strings.Join is linear. The part cap removes the quadratic-memory DoS vector, and I have no security concerns.

Tests

• Coverage is good. The only additions I'd suggest are the trailing dot after 5 parts and one assertion per caller path.

No blocking issues. The comment fix and the description update are the only changes I'd ask for.
· Branch: fix/parser-qualified-name-error-context

[ajitpratap0]

Merge train (batch 3): #627

• Rebase: onto main 1be0808 (after fix(ast): return CreateTable, FunctionDesc and trigger children as pointers (#617) #621, fix(linter,gosqlx): deterministic L006/L009 and Extract* result order (#618, #619) #625, fix(tokenizer,errors): release oversized lineStarts and bound error context on long lines (#614, #615) #626, fix(schema/postgres): correct index column order, INCLUDE/expression keys, partitioned indexes and key flags (#620) #622, fix(parser,ast,formatter): named window round-trip, GROUPS/EXCLUDE frames and INTERVAL bounds (#608, #609) #629, fix(parser): INSERT ... SELECT RETURNING, DEFAULT VALUES, MySQL INSERT ... SET and parenthesized sources (#610) #623, fix(parser,ast): ClickHouse parametric aggregates, FROM table functions and their rendering (#605, #606, #607) #628, fix(parser,ast,formatter): keep JOIN ... USING and accept parenthesized set operations (#603, #612) #630, fix(formatter,ast,parser): no inner semicolons; keep IGNORE NULLS, named args, ON DUPLICATE KEY, OUTPUT, SETTINGS (#602, #600) #631) with no conflicts. Head is a7120d6.
• Local gate on the rebased head: go vet ./..., golangci-lint (0 issues) and go test -race over every package except cbinding all pass.
• Adversarial and security pass: an Opus attack and refute round found 3 should-fix items. All three are fixed in 29aab08 and independently validated by Opus:
  • 627-1: Snowflake permissive CREATE STAGE/SEQUENCE/FILE FORMAT with a 6+ part name now returns E2004. It used to report success with a truncated RawSQL (CREATE SEQUENCE f).
  • 627-2: every non-FROM caller (JOIN, INSERT/UPDATE/DELETE, CREATE TABLE/VIEW/INDEX, DROP, TRUNCATE, REFRESH, SHOW/DESCRIBE/REPLACE, USE, ClickHouse MV TO) now passes the E2004 "more than 5 parts" error through, using a sentinel cause. Before, it was rewrapped as E2002 "expected table name".
  • 627-3: the BETWEEN/LIKE/REGEXP/IN-subquery operand wrappers no longer print a fake 1 | ) snippet; the inner error is kept as Cause. The parser: several E2002 errors pass hint text as the SQL source, printing a bogus context snippet #616 static guard now flags any sql argument other than "".
  • Validator's differential run (5688 runs: all testdata SQL plus generated name and operand cases × 9 dialects): no change outside these error paths and no panics. The new tests fail on the previous commit and pass on the head.
• Bot review:
  • The maxQualifiedNameParts comment now matches the constant, which is tied to maxColumnRefParts.
  • The E2002 wrapping follow-up and the Snowflake RawSQL truncation are fixed here rather than deferred (627-2, 627-1).
  • The guard is no longer literal-only (627-3).
  • a.b.c.d.e. (a trailing dot after 5 parts) reports E2004 "more than 5 parts" rather than "expected identifier". That is deliberate: the limit fires before the next token is read, which keeps hostile input bounded.
• Behaviour change: names with 6+ parts are rejected everywhere, including Snowflake permissive CREATE. This is recorded in the CHANGELOG via docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598.