[ajitpratap0]

Summary

Adds the W0-05 benchmark matrix from PLAN.md section 2.2, test files only, no production code changes.

• pkg/fingerprint: BenchmarkNormalize/<q>, BenchmarkFingerprint/<q> over the 11 portable queries; BenchmarkNormalizeScale/{in,values}/{10,100,1000}.
• pkg/formatter: BenchmarkRender/{compact,readable,lower,sql}/<q>, BenchmarkRenderRW/<dialect>/{nodialect,dialect}, BenchmarkRenderScale/{wide,in,values,and}/{10,100,1000}.
• pkg/gosqlx: BenchmarkFormatWithDialectRW/<dialect>, BenchmarkValidateRW/<dialect>/{small,medium,large}, BenchmarkHighlight/<q>.
• pkg/transform: BenchmarkTransformParse/<q>, BenchmarkTransformRoundTrip/{addwhere,addwhere_fromsql,limit,rename} plus a BenchmarkTransformRoundTripSQLServer sibling for the mssql column.
• pkg/sql/keywords: BenchmarkDetectDialect/<case> (8 detection routes plus a 64 KB script).
• internal/benchcorpus/scale.go: shared deterministic ScaleSQL(shape, n) generator.

Inputs come from internal/benchcorpus (W0-01). Statements are filtered once outside the timer so loops never time an error path. Each input set has a behaviour test (rendered styles differ and keep identifiers, compact equals SQL(), SQL Server limit renders as OFFSET/FETCH, buckets are sized correctly, spans are ordered and in bounds, detection results are pinned, fingerprints ignore literals).

Notes: the old no-subbenchmark BenchmarkNormalize/BenchmarkFingerprint (one join query) are replaced by the per-query sub-benchmarks, so their old names have no baseline. RenderScale was re-measured (default benchtime, and GOGC=off -cpu 1 at 10x/100x): wide/10, /100, /1000 are about 1.4, 12 and 110 us, linear at roughly 10x per step, so no super-linearity.

Test Plan

• task check: exit 0 (fmt, vet, golangci-lint 0 issues, race tests).
• task test:cbinding: exit 0.
• go test -count=1 ./pkg/compatibility/...: exit 0.
• go run honnef.co/go/tools/cmd/staticcheck@latest ./internal/benchcorpus ./pkg/fingerprint ./pkg/formatter ./pkg/transform ./pkg/gosqlx ./pkg/sql/keywords: exit 0.
• go test -run '^$' -bench <new benchmarks> -benchtime 10x/20x/50x in each package: all run and pass.
• No A/B benchstat: this PR adds benchmarks only.

Review fixes: transform round-trip benchmarks time a pure roundTripOnce (no tb.Helper in the loop); BenchmarkRenderRW releases its trees via b.Cleanup; renderRWOpts is shared and TestRenderRWDialectPathDiffers fails if the dialect variant does no extra work (verified by mutation).

Merge ordering: #903/#904 add same-named BenchmarkTransformParse/BenchmarkRender in-package; whichever lands after this PR must delete its copy and use these. W0-03 must not redefine rwDialects/rwValid.

Breaking Changes

None.

🤖 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	Nov 1, 2026 7:48am UTC

[github-actions]

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

---

Review of PR #911: benchmark matrix [W0-05]

This was a light review. I read scale.go and the first half of pkg/formatter/bench_matrix_test.go in full. I skimmed the rest from the diff stat and PR description. I did not run the tests or benchmarks.

Overall: The PR looks good. It only adds test files and internal/benchcorpus/scale.go, so it carries no production risk.

What I liked

• Setup outside the timer. Parsing and statement filtering happen before b.ResetTimer(), so the loops never time an error path.
• Pooled ASTs. defer ast.ReleaseAST(tree) follows the CLAUDE.md pooling rule.
• Sink variables. renderSink stops the compiler from discarding results.
• Behaviour tests. Each input set has a test that pins what it measures, such as compact equalling SQL() and bucket sizes. That keeps the benchmarks honest.
• ScaleSQL. It is deterministic and linear in n, and it panics on bad input, which fits a benchmark helper.

Minor suggestions (non-blocking)

1. ScaleValues literals. fmt.Sprintf("%d.5") gives float literals, which is fine. Building the strings per row with Sprintf is also fine at n ≤ 1000, since it runs once outside the timer. Callers should cache the SQL string rather than call ScaleSQL inside the loop. Please confirm the scale benchmarks do this.
2. Dialect filter. corpusDialects() and dialectTrees silently skip statements that fail to parse or render. If a corpus regression drops most of a dialect's statements, the benchmark would keep running on a much smaller set and the numbers would not be comparable. Consider b.Fatal when the surviving count falls below a floor, or log the kept/total count with b.Logf or b.ReportMetric.
3. Renamed baselines. BenchmarkNormalize and BenchmarkFingerprint lose their old names. You note this in the description. If any benchstat or CI baseline file references them, update it in the same wave.
4. RenderScale/wide/1000 (~65x wide/100). That ratio is clearly super-linear, so a follow-up issue to track it would be useful.

No bugs, security concerns or race issues found in the parts I read.

[github-actions]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[github-actions]

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

---

Review of PR #911: benchmark matrix [W0-05]

I read scale.go, the start of pkg/transform/bench_matrix_test.go, and the RenderScale benchmark setup. I skimmed the other files and did not run the tests or benchmarks.

Overall: This is a good PR. It changes only test files and the internal/benchcorpus helper, so it carries no production risk. I found no bugs, race issues or security concerns.

What works well

• ScaleSQL is deterministic and linear in n. It panics on bad input, which suits a benchmark helper.
• Setup happens outside the timer. For example, RenderScale parses ScaleSQL(shape, n) once per sub-benchmark before the loop (pkg/formatter/bench_matrix_test.go:206).
• roundTripOnce in pkg/transform/bench_matrix_test.go has no testing.TB calls inside the timed loop. The shared roundTripCases table drives both the benchmark and the test that pins its output.
• Each input set has a behaviour test, so the benchmarks measure what they claim to.

Suggestions (non-blocking)

1. Silent filtering. The dialect and portable-query filters skip statements that fail to parse or render. If the corpus regresses, the benchmark would run on a smaller set and its numbers would no longer be comparable. Add a floor check in the behaviour tests, or log the kept/total count with b.Logf.
2. Baselines. The old BenchmarkNormalize and BenchmarkFingerprint names are gone. Check that performance_baselines.json and any benchstat or CI baseline files don't reference them.
3. Merge ordering. perf(fingerprint,formatter,transform): tokenize into the pooled scratch buffer [W5-01] #903 and perf(formatter): render compact SELECT/INSERT/UPDATE/DELETE through SQL() [W5-02] #904 define same-named BenchmarkTransformParse and BenchmarkRender in-package, so whichever lands second will fail to compile on the duplicates. Your PR description already flags this. Consider noting it on those PRs too.
4. Scale linearity. An earlier comment on this PR claimed RenderScale/wide/1000 is about 65x wide/100. The PR description reports roughly 10x per step (1.4, 12 and 110 µs), so I don't see super-linearity. I haven't re-measured. If you want that nailed down, a -count=6 benchstat run would settle it.
5. ScaleValues literals. The %d.5 pattern produces float literals, which is fine. It is worth a one-line comment if that is intentional.