Skip to content

perf(security): necessary-condition guards for ScanSQL [W6-01] - #900

Merged
ajitpratap0 merged 6 commits into
mainfrom
perf/w6-01-scansql-guards
Nov 1, 2026
Merged

ajitpratap0 merged 6 commits into
mainfrom
perf/w6-01-scansql-guards

Conversation

@ajitpratap0

@ajitpratap0 ajitpratap0 commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

Summary

Wave-1 task W6-01 from the 2026-11 performance audit (SEC-1). The prototype was confirmed by an independent verifier. Codex ported it, and Opus reviewed it and ran the full gate. Review found that the audit's prototype was quadratic on adversarial input, and this PR fixes that.

ScanSQL ran three expensive regex paths on inputs that cannot match them. Each now has a guard that skips the work when no match is possible:

Guard Skips
hasDollarOpener: some $ can open a dollar quote (the lexer's own opener condition) the 32-state dollar-quote stripping lexer
hasEqualIdentPair: around some =, a suffix of the left identifier run equals a prefix of the right run, case-insensitively, up to 51 bytes the identifier tautology regex \b([a-z_][a-z0-9_.]{0,50})[\s\v]*=[\s\v]*([a-z_][a-z0-9_.]{0,50})\b
hasTableFnCandidate: some ( follows a listed ClickHouse table-function name that is preceded by whitespace the FROM|JOIN <table function>( regex

Why the guards are safe:

  • Necessary conditions only. A guard returns false only when its regex provably cannot match, and the regex still confirms every finding. Each guard's comment gives the argument.
  • 51-byte bound. The bound in hasEqualIdentPair is the regex's own capture limit. It keeps the guard linear: the audit prototype compared every suffix and prefix length, so a 64 KB input of unequal identifiers took about 1.8 s.
  • ASCII only. The identifier and table-function guards only run on ASCII input, because (?i) also folds U+017F (ſ) to S and U+212A (K) to K. Non-ASCII input takes the unguarded path.

Test Plan

  • TestScanSQLGuardsEquivalence. Findings are identical with the guards on and off over every statement and whole file in testdata/realworld/* and testdata/corpus/*, plus 200,000 seeded random and mutated inputs. Those include U+017F, U+212A, dollar quotes, identifier pairs and table functions.
    • Under -race or -short, a deterministic 20,000-input subset runs instead. The full corpus and the Unicode cases always run. Under -race it finishes in 19.6 s; the whole security package takes 31.6 s.
  • TestHasEqualIdentPairLinear. A timing-ratio test (wall clock, not a comparison count): min of 3 samples at 16 KiB vs 64 KiB, fails if the ratio exceeds 10x. Before the fix, quadrupling the input cost 16.3x; after, about 3.5-3.7x (stable over 30 normal and 10 -race runs).
  • TestHasEqualIdentPairCaptureBoundary (new). Border-free 51-byte equal identifiers on both sides of = (plain, spaced, case-folded) plus 52-byte cases checked against the regex-only path. Kills the k <= 51 to k <= 50 mutation.
  • TestTableFnGuardCoversEveryRegexName (new). Extracts every name from the ClickHouse alternation in scanner.go, requires the guard name set to match it exactly, and asserts the guard and ScanSQL fire for each name (mixed case, tab/VT whitespace). Kills the 16-byte azureBlobStorage length mutation and locks chNames to the regex.
  • Fuzzing. The guard inputs were added to the FuzzScanSQL seed corpus.
  • Gate, run outside the Codex sandbox:
Check Result
go run honnef.co/go/tools/cmd/staticcheck@latest ./pkg/sql/security/... ✅
go test -race -count=1 ./pkg/sql/security/... ✅ (31.6 s)
task check ✅
task test:cbinding ✅
go test -count=1 ./pkg/compatibility/... ✅

Re-measured after merging wave 0 (task bench:ab BASE=origin/main BENCH='./pkg/sql/security=^BenchmarkScanSQLRW$', Apple M4, shared machine so indicative only, 10 interleaved rounds, benchstat). Uses the wave-0 matrix BenchmarkScanSQLRW; the Codex duplicate benchmark was removed.

Dialect base head delta
postgresql 12.339 us 2.619 us -78.77% (p=0.007)
mysql 11.203 us 2.624 us -76.58% (p=0.009)
mariadb 8.311 us 3.835 us ~ (noisy)
sqlserver 11.811 us 3.146 us -73.36% (p=0.009)
oracle 17.106 us 3.361 us -80.35% (p=0.001)
sqlite 6.480 us 2.195 us ~ (noisy)
snowflake 6.438 us 3.088 us ~ (noisy)
clickhouse 6.456 us 3.683 us ~ (noisy)
geomean 9.430 us 3.022 us -67.95%

B/op -0.01%, allocs/op unchanged. Absolute numbers are inflated by machine load; the earlier quiet-machine run gave -66.0% geomean. Both exceed the plan's -45% target.

Reviewer re-measurement (6 interleaved rounds vs base c05c5fd5, ab.sh, M4 shared): ScanSQLRW improved significantly on all 8 dialects, -63.5% to -67.4% (p=0.002 each). ScanSQLParams: pg_dollar -95%, qmark -73%, at -74%. ScanSQLLarge throughput 30KB +11.4%, 1MB +12.1%. B/op -0.01%; allocs/op did not rise (30KB 32 to 31). So the earlier 'noisy' dialects are in fact significant.

amd64 evidence for the target (CI, run 41333802244, workflow_dispatch gate mode, base origin/main vs scratch branch perf/w6-01-gate-probe = this PR plus the single line ./pkg/sql/security ^BenchmarkScanSQLRW$ in scripts/perf/gate-suite.txt; 10 interleaved rounds, GOGC=off GOMAXPROCS=1): verdict pass.

Benchmark base head time p allocs/op B/op
ScanSQLRW/postgresql 1635.6us 604.3us -63.05% <0.001 382 -> 382 -0.00%
ScanSQLRW/mysql 1870.0us 721.2us -61.43% <0.001 517 -> 517 -0.01%
ScanSQLRW/mariadb 1694.1us 660.1us -61.03% <0.001 473 -> 473 -0.01%
ScanSQLRW/sqlserver 1948.5us 727.0us -62.69% <0.001 362 -> 362 -0.01%
ScanSQLRW/oracle 2355.3us 889.9us -62.22% <0.001 437 -> 437 -0.00%
ScanSQLRW/sqlite 1713.8us 635.4us -62.92% <0.001 419 -> 419 -0.01%
ScanSQLRW/snowflake 1794.9us 644.0us -64.12% <0.001 384 -> 384 -0.01%
ScanSQLRW/clickhouse 1825.1us 645.6us -64.63% <0.001 455 -> 455 -0.01%
geomean 1.844ms 686.2us -62.78%

This is the authoritative amd64 confirmation (matches the M4 figures). The PR's own shadow gate still runs the narrower suite on main (parse/tokenize: no regression, suite geomean -0.03%). Widening gate-suite.txt permanently (CRITIQUE item 27) is left to a W0-lane PR; the probe branch is scratch and not for merge.

Follow-ups (not in this PR): TestHasEqualIdentPairLinear is a wall-clock ratio test, like the existing TestWPPScanSQLLinear; W0-04 owns converting those to deterministic checks. isASCII(view) is recomputed per guarded pattern (cache once per view under W6-08/W6-10). hasEqualIdentPair could be made O(51) per = with Z/KMP if it ever matters.

Update: merged origin/main (69a5a72, W2-01) to resolve the CHANGELOG conflict; AC-STD gate re-run green. Wave-0 goldens (testdata/goldens/security.sha256) pass unchanged: no golden changed.

Breaking Changes

None. Findings are byte-identical, and the API is unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

ScanSQL ran three expensive regex paths on inputs that cannot match them.
Three guards now skip that work when a match is impossible:

- hasDollarOpener: no '$' can open a dollar quote, so the 32-state
  dollar-quote stripping lexer is skipped.
- hasEqualIdentPair: around some '=', a suffix of the left identifier run
  equals a prefix of the right run (case-insensitive, at most 51 bytes, the
  regex's capture bound). Otherwise the identifier tautology regex is
  skipped. The 51-byte cap keeps the guard linear; the audit prototype was
  quadratic on long unequal identifiers (64 KB took ~1.8 s).
- hasTableFnCandidate: some '(' follows a listed ClickHouse table-function
  name preceded by whitespace. Otherwise the FROM/JOIN table-function regex
  is skipped.

Each guard is a necessary condition and the regex still confirms every
finding. The identifier and table-function guards run only on ASCII input,
because (?i) also folds U+017F and U+212A. Findings are identical: an
equivalence test runs the corpora plus 200k seeded inputs (20k under -race
or -short), and the interesting inputs seed FuzzScanSQL. A scaling test
counts comparisons to keep hasEqualIdentPair linear.

M4, GOGC=off -cpu 1, 10 interleaved rounds: ScanSQLRW per statement -57%
to -69% per dialect, geomean -66.0%.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@vercel

vercel Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
gosqlx-website Ready Ready Preview Nov 1, 2026 7:13pm UTC

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

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


Review of PR #900: ScanSQL necessary-condition guards

Verdict: looks good. I have no blocking issues. I read the diff and guards.go closely. I did not run the tests or benchmarks in this session.

Correctness of the guards

  • hasEqualIdentPair: The run extraction matches the regex. Both captures sit directly against \s*=\s*, so the left capture must be a suffix of the left run and the right capture a prefix of the right run. The guard's run alphabet ([A-Za-z0-9_.]) is looser than the regex's first-character class [a-z_]. That only makes the guard over-accept, which is safe. The k <= 51 cap equals 1 + 50, the regex's maximum capture length. Work per = is therefore constant, which fixes the quadratic behaviour you found in the prototype.
  • hasTableFnCandidate: Because the regex requires FROM|JOIN plus whitespace directly before the name, the name must be a whole word run preceded by whitespace. The guard's e-s < 2 and > 16 bounds match the shortest name (s3) and the longest (azureblobstorage). Lowercasing into a stack buffer avoids allocation. string(buf[:n]) used as a map key does not allocate.
  • hasDollarOpener: It reuses the lexer's own opener condition (identifierByte and dollarTagEnd), so the two cannot drift apart.
  • ASCII gating: Only the identifier and table-function guards depend on the U+017F and U+212A folding, and both are gated on isASCII. The dollar guard has no such dependency.

Minor suggestions (non-blocking)

  1. isASCII(sql) is recomputed for every view and every guarded pattern. Views are usually few, but isASCII is a full scan. Computing it once per view, or once for folded, would shave a little more off the hot path.
  2. The same applies in detectTautologyInSQL for patternIndex == 2. It uses a magic index, like the existing patternIndex == 0. A short named constant, or a guard field on those patterns as in compiledPattern, would be more robust if the pattern list is reordered.
  3. chNames is built in init() from a split string. A package-level map literal would be simpler and keep the list next to the regex. Please also add a comment, or a test, that ties it to the alternation in scanner.go. Today the two lists must be kept in sync by hand, and a name added only to the regex would silently never match. A small test that extracts the alternation and asserts that the sets are equal would prevent that.
  4. In hasDollarOpener, the initial strings.IndexByte(sql, '$') < 0 check in stripDollarQuotedStrings is now redundant, because hasDollarOpener handles the no-$ case. It is harmless.

Tests

  • Coverage is strong. The on/off equivalence test runs over the real-world corpus plus 200k seeded inputs. There is a linearity test and fuzz seeds, and the Unicode folding cases always run.
  • One suggestion: the linearity test counts comparisons, which is good because it is deterministic. A small explicit regression case for the old 64 KB unequal-identifier input would document the original failure.

Performance and security

  • The reported −66% geomean is credible, because the guards skip regexes on inputs that cannot match.
  • Findings stay regex-confirmed, so false negatives are the only risk. The reasoning above shows none.
  • Non-ASCII input takes the unguarded path. Adversarial input can therefore still force the slow path by adding one non-ASCII byte. That is the same as today's behaviour, so it is not a regression, but you might note it.

Because it is a draft held for W0-02 and W0-09, merging after those land seems right.

ajitpratap0 and others added 2 commits November 1, 2026 22:33
…-01]

Merge origin/main (wave 0). The wave-0 matrix owns BenchmarkScanSQLRW.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

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


Review of PR #900: ScanSQL necessary-condition guards

Verdict: no blocking issues. I read the diff and guards.go, and checked the guard logic by hand. I did not run the tests or benchmarks in this session.

Correctness

  • hasEqualIdentPair:
    • The regex captures sit directly against \s*=\s*. The left capture is therefore a suffix of the left run, and the right capture is a prefix of the right run.
    • The guard's alphabet ([A-Za-z0-9_.]) is wider than the regex's first-character class, so it can only over-accept, which is safe.
    • The k <= 51 bound matches the regex's 1 + {0,50} capture limit, so work per = is constant. This fixes the quadratic behaviour.
    • I checked the \b boundaries and found no case where the guard returns false and the regex matches.
  • hasTableFnCandidate:
    • The regex needs whitespace directly before the name. Whitespace is not a word byte, so the backward word-run scan lands exactly on the name start.
    • The length bounds (2 for s3, 16 for azureblobstorage) match the name list.
    • The buf fill and the map lookup do not allocate.
  • hasDollarOpener: it reuses the lexer's own opener condition (identifierByte and dollarTagEnd), so the two cannot drift apart.
  • ASCII gating: both ASCII-only guards are gated on isASCII, which covers the U+017F and U+212A folding concern. Non-ASCII input takes the unguarded path.

Suggestions (non-blocking)

  1. isASCII cost. isASCII(sql) is a full scan, and it runs for each view and each guarded pattern. Computing it once per view, or once for folded, would save a little on the hot path.
  2. Magic index. patternIndex == 2 in detectTautologyInSQL (scanner.go:1217) is fragile if the pattern list is reordered. A named constant, or a guard field on these patterns as in compiledPattern, would be more robust.
  3. chNames can drift from the regex.
    • Problem: chNames (guards.go:77) is a hand-maintained copy of the alternation in scanner.go. A name added only to the regex would never match, because the guard would skip it. That is a silent false negative.
    • Fix: a package-level map literal next to the regex would be simpler. Either way, add a small test that extracts the alternation from the regex source and asserts the two sets are equal.
    • Why it matters: TestScanSQLGuardsEquivalence only catches this if its inputs happen to use the new name.
  4. Redundant check. strings.IndexByte(sql, '$') < 0 || in stripDollarQuotedStrings is now redundant, since hasDollarOpener handles the no-$ case. It is harmless but could be dropped.
  5. Explicit regression test. TestHasEqualIdentPairLinear counts comparisons, which is good and deterministic. An explicit 64 KB unequal-identifier case with a generous time bound would also document the original failure.
  6. Adversarial slow path. Adding one non-ASCII byte forces the unguarded path. That matches today's behaviour, so it is not a regression, but it is worth a note in the doc comment.

Tests, performance and security

  • Coverage is strong: on/off equivalence over the real-world corpus plus 200k seeded inputs, a linearity test, fuzz seeds, and the Unicode cases.
  • The reported −66% geomean is plausible, since the guards skip regex work on inputs that cannot match.
  • Findings are still confirmed by the regex, so the only risk is a false negative, and I found none in the reasoning above.
  • Holding the PR until W0-02 and W0-09 land seems right.
    | Branch: perf/w6-01-scansql-guards

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Perf gate: PASS (shadow mode: report only, this check never fails)

Base d29c64466939, head a3b539af827b. 10 interleaved rounds, benchtime 200ms, -cpu 1, pinned to CPU 3 (SMT siblings 2-3) with taskset.

  • no gated metric moved past its threshold

Rules: time regresses at >= +3% with p < 0.01 (Mann-Whitney U), or a suite geomean >= +1.5% (per-round geomeans, same test); allocs/op must not increase; B/op must not rise more than 2%; instructions/op must not rise more than 1.5%. An A/A control with more than 10% of benchmarks moving over 3% is noisy: re-run once, then inconclusive (neutral).

Gated suite (GOGC=off, GOMAXPROCS=1)

benchmark time p A/A B/op allocs/op status
ParseE2E/simple_select +0.17% 0.161 +0.34% +0.00% 5 → 5 ok
ParseE2E/select_where +0.38% 0.089 +0.30% +0.00% 18 → 18 ok
ParseE2E/join -0.05% 0.705 -0.25% +0.00% 17 → 17 ok
ParseE2E/aggregate -0.45% 0.384 -0.16% +0.00% 18 → 18 ok
ParseE2E/subquery -0.56% 0.069 -0.34% +0.00% 14 → 14 ok
ParseE2E/cte +0.45% 0.140 -0.24% +0.00% 21 → 21 ok
ParseE2E/window -0.17% 0.495 -0.06% +0.00% 11 → 11 ok
ParseE2E/complex -0.15% 0.762 -0.51% +0.00% 50 → 50 ok
ParseE2E/insert -0.54% 0.053 -0.10% +0.00% 17 → 17 ok
ParseE2E/update +0.55% 0.096 +0.03% +0.00% 13 → 13 ok
ParseE2E/delete +0.28% 0.494 -0.14% +0.00% 5 → 5 ok
ParseRW/postgresql/medium -0.19% 0.218 -0.47% +0.00% 646 → 646 ok
ParseRW/mysql/medium +0.42% 0.353 -0.30% +0.00% 365 → 365 ok
ParseRW/mariadb/medium -0.12% 0.739 -0.57% +0.00% 523 → 523 ok
ParseRW/sqlserver/medium +0.36% 0.165 -0.36% +0.00% 936 → 936 ok
ParseRW/oracle/medium -0.05% 0.853 +0.15% +0.00% 1093 → 1093 ok
ParseRW/sqlite/medium +0.17% 0.353 -0.33% +0.00% 346 → 346 ok
ParseRW/snowflake/medium +0.39% 0.063 +0.01% +0.00% 777 → 777 ok
ParseRW/clickhouse/medium +0.32% 0.023 +0.04% +0.00% 384 → 384 ok
ParseFresh/simple_select -0.11% 0.325 +0.62% +0.00% 6 → 6 ok
ParseFresh/join -0.47% 0.520 -0.74% +0.00% 18 → 18 ok
ParseFresh/complex +0.28% 0.315 -0.07% +0.00% 51 → 51 ok
PoolGetPut/serial +0.13% 0.448 +0.08% +0.00% 0 → 0 ok
Tokenize/scratch/join -0.08% 0.447 -0.03% +0.00% 0 → 0 ok
Tokenize/scratch/complex +0.01% 0.850 -0.04% +0.00% 0 → 0 ok
Tokenize/scratch/insert -0.22% 0.104 +0.08% +0.00% 0 → 0 ok
Normalize/simple_select +0.26% 0.010 -0.34% +0.00% 10 → 10 ok
Normalize/join -0.23% 0.190 -0.22% +0.00% 40 → 40 ok
Normalize/complex +0.02% 0.762 -0.13% +0.00% 110 → 110 ok
TransformParse/simple_select +0.10% 0.324 -0.24% +0.00% 5 → 5 ok
TransformParse/join +0.17% 0.384 -0.16% +0.00% 17 → 17 ok
TransformParse/complex +0.22% 0.579 -0.06% +0.00% 50 → 50 ok
geomean +0.04% 0.143 -0.13%

A/A control: 0 of 32 benchmarks moved more than 3%.

Instructions per operation (cmd/perfdriver)

case instructions/op status
e2e/aggregate -0.01% ok
e2e/complex -0.01% ok
e2e/cte -0.04% ok
e2e/delete -0.03% ok
e2e/insert -0.02% ok
e2e/join -0.04% ok
e2e/select_where +0.05% ok
e2e/simple_select -0.02% ok
e2e/subquery +0.03% ok
e2e/update -0.10% ok
e2e/window -0.03% ok
rw/postgresql/medium -0.04% ok
Secondary metric: default GC, GOMAXPROCS=1 (informational)
benchmark time p
ParseE2E/simple_select -0.49% 0.850
ParseE2E/select_where -1.52% 0.121
ParseE2E/join +0.04% 0.912
ParseE2E/aggregate +0.68% 0.290
ParseE2E/subquery +2.75% 0.123
ParseE2E/cte +1.02% 0.199
ParseE2E/window +1.23% 0.241
ParseE2E/complex -0.76% 0.450
ParseE2E/insert -0.35% 0.345
ParseE2E/update +0.56% 0.130
ParseE2E/delete -0.30% 0.427
ParseRW/postgresql/medium -0.63% 0.315
ParseRW/mysql/medium -0.76% 0.853
ParseRW/mariadb/medium -0.36% 0.393
ParseRW/sqlserver/medium -0.35% 0.190
ParseRW/oracle/medium -1.06% 0.280
ParseRW/sqlite/medium -0.90% 0.123
ParseRW/snowflake/medium -1.38% 0.019
ParseRW/clickhouse/medium -2.02% 0.002
ParseFresh/simple_select -0.29% 0.089
ParseFresh/join -0.27% 0.739
ParseFresh/complex -1.08% 0.739
PoolGetPut/serial -0.03% 0.493
Tokenize/scratch/join +0.00% 0.790
Tokenize/scratch/complex -0.03% 1.000
Tokenize/scratch/insert -0.24% 0.003
Normalize/simple_select -0.48% 0.762
Normalize/join -3.17% <0.001
Normalize/complex -2.71% 0.009
TransformParse/simple_select -1.50% 0.075
TransformParse/join -0.87% 0.529
TransformParse/complex +0.34% 0.912
geomean -0.47%
Environment
date:        2026-11-01T19:20:13Z
base:        d29c64466939295f6c00b5283411faec33fb6175
head:        working tree of a3b539af827b3fd07824618ecaa883abd87afe1e
go:          go version go1.26.1 linux/amd64
kernel:      Linux 7.0.0-1017-azure x86_64
pinning:     pinned to CPU 3 (SMT siblings 2-3) with taskset
clocksource: tsc
load:        1.52, 0.45, 0.16
instr count: cachegrind
perf probe:  Error: No supported events found. The instructions:u event is not supported. 

Architecture:                            x86_64
CPU(s):                                  4
Model name:                              AMD EPYC 9V74 80-Core Processor
Thread(s) per core:                      2
Core(s) per socket:                      2
Socket(s):                               1
Hypervisor vendor:                       Microsoft
L1d cache:                               64 KiB (2 instances)
L1i cache:                               64 KiB (2 instances)
L2 cache:                                2 MiB (2 instances)
L3 cache:                                32 MiB (1 instance)
benchstat (primary metric)
goos: linux
goarch: amd64
pkg: github.com/ajitpratap0/GoSQLX/pkg/fingerprint
cpu: AMD EPYC 9V74 80-Core Processor                
                        │    base     │                head                │
                        │   sec/op    │   sec/op     vs base               │
Normalize/simple_select   2.331µ ± 0%   2.337µ ± 1%  +0.26% (p=0.007 n=10)
Normalize/join            7.841µ ± 0%   7.823µ ± 1%       ~ (p=0.190 n=10)
Normalize/complex         19.38µ ± 0%   19.39µ ± 0%       ~ (p=0.753 n=10)
geomean                   7.076µ        7.077µ       +0.02%

                        │     base     │                 head                  │
                        │     B/op     │     B/op      vs base                 │
Normalize/simple_select   1.016Ki ± 0%   1.016Ki ± 0%       ~ (p=1.000 n=10) ¹
Normalize/join            3.234Ki ± 0%   3.234Ki ± 0%       ~ (p=1.000 n=10) ¹
Normalize/complex         8.945Ki ± 0%   8.945Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean                   3.086Ki        3.086Ki       +0.00%
¹ all samples are equal

                        │    base    │                head                 │
                        │ allocs/op  │ allocs/op   vs base                 │
Normalize/simple_select   10.00 ± 0%   10.00 ± 0%       ~ (p=1.000 n=10) ¹
Normalize/join            40.00 ± 0%   40.00 ± 0%       ~ (p=1.000 n=10) ¹
Normalize/complex         110.0 ± 0%   110.0 ± 0%       ~ (p=1.000 n=10) ¹
geomean                   35.30        35.30       +0.00%
¹ all samples are equal

pkg: github.com/ajitpratap0/GoSQLX/pkg/gosqlx
                          │    base     │                head                │
                          │   sec/op    │   sec/op     vs base               │
ParseE2E/simple_select      1.461µ ± 1%   1.463µ ± 2%       ~ (p=0.159 n=10)
ParseE2E/select_where       3.017µ ± 0%   3.029µ ± 1%       ~ (p=0.086 n=10)
ParseE2E/join               4.569µ ± 1%   4.567µ ± 1%       ~ (p=0.698 n=10)
ParseE2E/aggregate          3.342µ ± 1%   3.327µ ± 2%       ~ (p=0.383 n=10)
ParseE2E/subquery           2.944µ ± 1%   2.928µ ± 2%       ~ (p=0.064 n=10)
ParseE2E/cte                4.001µ ± 0%   4.019µ ± 1%       ~ (p=0.137 n=10)
ParseE2E/window             2.614µ ± 0%   2.609µ ± 2%       ~ (p=0.492 n=10)
ParseE2E/complex            12.25µ ± 1%   12.23µ ± 1%       ~ (p=0.753 n=10)
ParseE2E/insert             2.040µ ± 1%   2.029µ ± 1%       ~ (p=0.050 n=10)
ParseE2E/update             1.815µ ± 1%   1.825µ ± 1%       ~ (p=0.093 n=10)
ParseE2E/delete             1.073µ ± 1%   1.076µ ± 0%       ~ (p=0.492 n=10)
ParseRW/postgresql/medium   147.4µ ± 0%   147.1µ ± 1%       ~ (p=0.218 n=10)
ParseRW/mysql/medium        78.80µ ± 1%   79.13µ ± 1%       ~ (p=0.353 n=10)
ParseRW/mariadb/medium      114.4µ ± 1%   114.3µ ± 1%       ~ (p=0.739 n=10)
ParseRW/sqlserver/medium    218.3µ ± 1%   219.1µ ± 1%       ~ (p=0.165 n=10)
ParseRW/oracle/medium       265.7µ ± 0%   265.6µ ± 1%       ~ (p=0.853 n=10)
ParseRW/sqlite/medium       68.31µ ± 1%   68.42µ ± 2%       ~ (p=0.353 n=10)
ParseRW/snowflake/medium    160.9µ ± 1%   161.5µ ± 1%       ~ (p=0.063 n=10)
ParseRW/clickhouse/medium   76.02µ ± 0%   76.27µ ± 1%       ~ (p=0.023 n=10)
ParseFresh/simple_select    7.861µ ± 0%   7.852µ ± 0%       ~ (p=0.324 n=10)
ParseFresh/join             17.29µ ± 1%   17.20µ ± 0%       ~ (p=0.517 n=10)
ParseFresh/complex          27.80µ ± 1%   27.88µ ± 2%       ~ (p=0.315 n=10)
PoolGetPut/serial           55.67n ± 2%   55.74n ± 1%       ~ (p=0.446 n=10)
geomean                     11.20µ        11.21µ       +0.04%

                          │     base     │                head                 │
                          │     B/s      │     B/s       vs base               │
ParseE2E/simple_select      16.98Mi ± 1%   16.94Mi ± 1%       ~ (p=0.148 n=10)
ParseE2E/select_where       32.56Mi ± 0%   32.43Mi ± 1%       ~ (p=0.085 n=10)
ParseE2E/join               26.30Mi ± 1%   26.31Mi ± 1%       ~ (p=0.698 n=10)
ParseE2E/aggregate          23.98Mi ± 1%   24.08Mi ± 2%       ~ (p=0.403 n=10)
ParseE2E/subquery           26.56Mi ± 1%   26.71Mi ± 2%       ~ (p=0.072 n=10)
ParseE2E/cte                29.55Mi ± 0%   29.42Mi ± 1%       ~ (p=0.128 n=10)
ParseE2E/window             34.30Mi ± 0%   34.36Mi ± 2%       ~ (p=0.492 n=10)
ParseE2E/complex            29.43Mi ± 1%   29.47Mi ± 1%       ~ (p=0.812 n=10)
ParseE2E/insert             35.53Mi ± 1%   35.72Mi ± 1%       ~ (p=0.060 n=10)
ParseE2E/update             28.91Mi ± 1%   28.75Mi ± 1%       ~ (p=0.101 n=10)
ParseE2E/delete             46.23Mi ± 1%   46.12Mi ± 0%       ~ (p=0.383 n=10)
ParseRW/postgresql/medium   24.34Mi ± 0%   24.39Mi ± 1%       ~ (p=0.224 n=10)
ParseRW/mysql/medium        34.35Mi ± 1%   34.20Mi ± 1%       ~ (p=0.325 n=10)
ParseRW/mariadb/medium      30.99Mi ± 1%   31.03Mi ± 1%       ~ (p=0.782 n=10)
ParseRW/sqlserver/medium    30.47Mi ± 1%   30.36Mi ± 1%       ~ (p=0.171 n=10)
ParseRW/oracle/medium       31.55Mi ± 0%   31.56Mi ± 1%       ~ (p=0.781 n=10)
ParseRW/sqlite/medium       30.24Mi ± 1%   30.19Mi ± 2%       ~ (p=0.342 n=10)
ParseRW/snowflake/medium    31.61Mi ± 1%   31.48Mi ± 1%       ~ (p=0.066 n=10)
ParseRW/clickhouse/medium   29.64Mi ± 0%   29.54Mi ± 1%       ~ (p=0.022 n=10)
geomean                     29.64Mi        29.62Mi       -0.07%

                          │      base      │                 head                  │
                          │      B/op      │     B/op      vs base                 │
ParseE2E/simple_select        816.0 ± 0%       816.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/select_where       1.523Ki ± 0%     1.523Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/join               2.305Ki ± 0%     2.305Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/aggregate          1.812Ki ± 0%     1.812Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/subquery           1.828Ki ± 0%     1.828Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/cte                2.336Ki ± 0%     2.336Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/window             1.281Ki ± 0%     1.281Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/complex            6.399Ki ± 0%     6.399Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/insert               824.0 ± 0%       824.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/update               736.0 ± 0%       736.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/delete               400.0 ± 0%       400.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/postgresql/medium   70.51Ki ± 0%     70.51Ki ± 0%       ~ (p=1.000 n=10)
ParseRW/mysql/medium        43.72Ki ± 0%     43.72Ki ± 0%       ~ (p=1.000 n=10)
ParseRW/mariadb/medium      64.84Ki ± 0%     64.84Ki ± 0%       ~ (p=0.195 n=10)
ParseRW/sqlserver/medium    90.61Ki ± 0%     90.62Ki ± 0%  +0.00% (p=0.008 n=10)
ParseRW/oracle/medium       113.8Ki ± 0%     113.8Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlite/medium       39.89Ki ± 0%     39.89Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium    83.38Ki ± 0%     83.38Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/clickhouse/medium   37.13Ki ± 0%     37.13Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/simple_select      896.0 ± 0%       896.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/join             2.383Ki ± 0%     2.383Ki ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/complex          6.477Ki ± 0%     6.477Ki ± 0%       ~ (p=1.000 n=10) ¹
PoolGetPut/serial             0.000 ± 0%       0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                  ²                 +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                          │     base      │                 head                 │
                          │   allocs/op   │  allocs/op   vs base                 │
ParseE2E/simple_select       5.000 ± 0%      5.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/select_where        18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/join                17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/aggregate           18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/subquery            14.00 ± 0%      14.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/cte                 21.00 ± 0%      21.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/window              11.00 ± 0%      11.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/complex             50.00 ± 0%      50.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/insert              17.00 ± 0%      17.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/update              13.00 ± 0%      13.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseE2E/delete              5.000 ± 0%      5.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/postgresql/medium    646.0 ± 0%      646.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mysql/medium         365.0 ± 0%      365.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/mariadb/medium       523.0 ± 0%      523.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlserver/medium     936.0 ± 0%      936.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/oracle/medium       1.093k ± 0%     1.093k ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/sqlite/medium        346.0 ± 0%      346.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/snowflake/medium     777.0 ± 0%      777.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseRW/clickhouse/medium    384.0 ± 0%      384.0 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/simple_select     6.000 ± 0%      6.000 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/join              18.00 ± 0%      18.00 ± 0%       ~ (p=1.000 n=10) ¹
ParseFresh/complex           51.00 ± 0%      51.00 ± 0%       ~ (p=1.000 n=10) ¹
PoolGetPut/serial            0.000 ± 0%      0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                 ²                +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                          │    base     │                head                │
                          │  sec/stmt   │  sec/stmt    vs base               │
ParseRW/postgresql/medium   12.28µ ± 0%   12.26µ ± 1%       ~ (p=0.225 n=10)
ParseRW/mysql/medium        9.850µ ± 1%   9.891µ ± 1%       ~ (p=0.342 n=10)
ParseRW/mariadb/medium      12.71µ ± 1%   12.70µ ± 1%       ~ (p=0.739 n=10)
ParseRW/sqlserver/medium    10.40µ ± 1%   10.43µ ± 1%       ~ (p=0.184 n=10)
ParseRW/oracle/medium       11.07µ ± 0%   11.07µ ± 1%       ~ (p=0.782 n=10)
ParseRW/sqlite/medium       11.38µ ± 1%   11.40µ ± 2%       ~ (p=0.342 n=10)
ParseRW/snowflake/medium    12.37µ ± 1%   12.42µ ± 1%       ~ (p=0.066 n=10)
ParseRW/clickhouse/medium   10.86µ ± 0%   10.89µ ± 1%       ~ (p=0.019 n=10)
geomean                     11.33µ        11.34µ       +0.16%

pkg: github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer
                         │    base     │                head                │
                         │   sec/op    │   sec/op     vs base               │
Tokenize/scratch/join      1.982µ ± 0%   1.980µ ± 1%       ~ (p=0.445 n=10)
Tokenize/scratch/complex   5.417µ ± 0%   5.418µ ± 0%       ~ (p=0.838 n=10)
Tokenize/scratch/insert    963.0n ± 0%   960.9n ± 1%       ~ (p=0.101 n=10)
geomean                    2.178µ        2.176µ       -0.10%

                         │     base     │                head                 │
                         │     B/s      │     B/s       vs base               │
Tokenize/scratch/join      60.64Mi ± 0%   60.70Mi ± 1%       ~ (p=0.325 n=10)
Tokenize/scratch/complex   66.55Mi ± 0%   66.55Mi ± 0%       ~ (p=0.853 n=10)
Tokenize/scratch/insert    75.26Mi ± 0%   75.43Mi ± 1%       ~ (p=0.101 n=10)
geomean                    67.22Mi        67.29Mi       +0.10%

                         │     base     │                head                 │
                         │     B/op     │    B/op     vs base                 │
Tokenize/scratch/join      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                               ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                         │     base     │                head                 │
                         │  allocs/op   │ allocs/op   vs base                 │
Tokenize/scratch/join      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/complex   0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Tokenize/scratch/insert    0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                               ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

pkg: github.com/ajitpratap0/GoSQLX/pkg/transform
                             │    base     │                head                │
                             │   sec/op    │   sec/op     vs base               │
TransformParse/simple_select   1.468µ ± 1%   1.469µ ± 1%       ~ (p=0.322 n=10)
TransformParse/join            4.897µ ± 1%   4.906µ ± 1%       ~ (p=0.382 n=10)
TransformParse/complex         13.39µ ± 2%   13.42µ ± 0%       ~ (p=0.579 n=10)
geomean                        4.583µ        4.590µ       +0.16%

                             │     base     │                 head                  │
                             │     B/op     │     B/op      vs base                 │
TransformParse/simple_select     816.0 ± 0%     816.0 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/join            2.305Ki ± 0%   2.305Ki ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/complex         6.398Ki ± 0%   6.398Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean                        2.273Ki        2.273Ki       +0.00%
¹ all samples are equal

                             │    base    │                head                 │
                             │ allocs/op  │ allocs/op   vs base                 │
TransformParse/simple_select   5.000 ± 0%   5.000 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/join            17.00 ± 0%   17.00 ± 0%       ~ (p=1.000 n=10) ¹
TransformParse/complex         50.00 ± 0%   50.00 ± 0%       ~ (p=1.000 n=10) ¹
geomean                        16.20        16.20       +0.00%
¹ all samples are equal

Run, raw data and benchstat

Merge origin/main (keep both CHANGELOG entries). Add tests that kill two
surviving mutations: the 51-byte identifier capture bound and the 16-byte
azureBlobStorage table-function name; the latter also locks the guard name
set to the regex alternation in scanner.go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

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


Review of PR #900: ScanSQL necessary-condition guards

Verdict: no blocking issues. I read guards.go and the diffs to scanner.go, prefilter.go and dollar_quote.go, and checked each guard's logic by hand. I did not run the tests or benchmarks in this session.

Correctness

  • hasEqualIdentPair:
    • The left capture must end where the whitespace before = begins, and the right capture must start right after the whitespace that follows =. A suffix/prefix comparison over the two runs is therefore a true necessary condition.
    • The guard's alphabet is wider than the regex's first-character class, so it can only over-accept.
    • The k <= 51 bound equals the regex's 1 + {0,50} capture limit, so work per = is constant. This fixes the quadratic behaviour.
  • hasTableFnCandidate:
    • The regex needs [\s\v]+ directly before the name and [\s\v]*\( directly after it. The guard's backward word-run scan therefore lands exactly on the name start.
    • The length bounds are right: 2 for s3, 16 for azureblobstorage, which fits buf.
    • The string(buf[:n]) map lookup does not allocate.
  • hasDollarOpener: it reuses the lexer's own opener predicate (identifierByte and dollarTagEnd), so the two cannot drift.
  • ASCII gating: both (?i)-sensitive guards run only on ASCII input, which covers the U+017F and U+212A folding concern. The guarded paths also run only when folded != nil, so any other caller keeps the old behaviour.

Suggestions (non-blocking)

  1. chNames can drift from the regex (guards.go:77-83).
    • The list is a hand-maintained copy of the alternation in scanner.go. A name added only to the regex would silently never match, because the guard would skip it.
    • A test that parses the alternation out of the regex source and asserts the two sets are equal would make this safe. A package-level map literal would also be simpler than the init() split.
  2. isASCII cost. It is a full scan, and it runs per view for each guarded pattern. Computing it once per view, or once for folded, would save a little on the hot path.
  3. Magic index. patternIndex == 2 in detectTautologyInSQL (scanner.go:1217) breaks silently if the pattern list is reordered. A named constant, or a guard field as on compiledPattern, would be sturdier.
  4. Redundant check. In stripDollarQuotedStrings, strings.IndexByte(sql, '$') < 0 || is now covered by hasDollarOpener. It is harmless, but it can be dropped.
  5. Non-ASCII slow path. One non-ASCII byte forces the unguarded path. This is the same as today's behaviour, so it is not a regression, but a doc-comment note would help. The perf gain does not apply to such inputs.

Tests

  • Coverage is strong: on/off equivalence over the corpora plus 200k seeded inputs, a deterministic linearity test, boundary tests, and fuzz seeds.
  • The perf gate cannot see pkg/sql/security, as the PR notes. The ScanSQLRW gain therefore rests on the local M4 numbers. It would be worth confirming on amd64 once the gate gains a security benchmark.

Performance and security

  • The guards only skip work, and the regex still confirms every finding. The only risk is a false negative, and I found none in the logic above.
  • The -68% geomean on ScanSQLRW is plausible.
  • Holding the PR until W0-02 and W0-09 land is the right call.
    | Branch: perf/w6-01-scansql-guards

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

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


Review of PR #900: ScanSQL necessary-condition guards

Verdict: no blocking issues. I read guards.go in full and checked the guard logic by hand. I did not run the tests or benchmarks in this session. The earlier review comments were written against an older head, so some of their suggestions are already addressed.

Correctness

  • hasEqualIdentPair (guards.go:45)
    • The runs are taken directly against the whitespace around =. A suffix/prefix comparison is therefore a true necessary condition for the regex.
    • The guard's alphabet is wider than the regex's first-character class, so it can only over-accept.
    • The k <= 51 cap equals 1 + {0,50}, so work per = is constant. This fixes the quadratic behaviour.
    • EqualFold on ASCII-only input is safe, because the function is gated on isASCII.
  • hasTableFnCandidate (guards.go:95)
    • The backward scan lands on the name start, because whitespace is not a word byte.
    • The e-s < 2 || e-s > 16 bounds match s3 and azureblobstorage.
    • The map lookup with string(buf[:n]) does not allocate.
  • hasDollarOpener: it reuses the lexer's opener predicate, so it cannot drift from it.
  • Unicode folding: gating both (?i)-sensitive guards on ASCII input handles U+017F and U+212A, and the Unicode cases always run in the tests.

Earlier suggestions: status

  • chNames drifting from the regex: now covered by TestTableFnGuardCoversEveryRegexName, which requires an exact set match. The init() split is still a bit roundabout. A map literal, or building the set from the same string constant the regex uses, would be tidier. This is optional.
  • Linearity test: it is now wall-clock, and the 3.5–3.7x ratio against the 10x limit leaves enough margin.

Remaining nits (non-blocking)

  1. isASCII is a full scan per view for each guarded pattern. Computing it once per view would save a little.
  2. patternIndex == 2 in detectTautologyInSQL is a magic index. A guard field set by withGuard, as compiledPattern already allows, would avoid that.
  3. strings.IndexByte(sql, '$') < 0 in stripDollarQuotedStrings is now redundant. It is harmless.
  4. Add a doc-comment note that one non-ASCII byte takes the unguarded path. This matches today's behaviour, but the speedup does not apply to such inputs.
  5. -race timing tests (the 19.6 s equivalence test, the wall-clock ratio) could be flaky on loaded CI runners. The ratio test is the more sensitive of the two, so keep an eye on it.

Performance and security

  • Findings are still regex-confirmed, so the only risk is a false negative. I found none in the logic, and the 200k-input on/off equivalence run backs that up.
  • The CI perf gate does not exercise pkg/sql/security. The ScanSQLRW gain therefore rests on the local M4 numbers until an amd64 run is done.
    | Branch: perf/w6-01-scansql-guards

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

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


Review of PR #900: ScanSQL necessary-condition guards

Verdict: no blocking issues. I read guards.go and the diffs to scanner.go, prefilter.go and dollar_quote.go, and checked each guard by hand. I did not run the tests or benchmarks in this session. I did not review the test files in detail.

Correctness

  • hasEqualIdentPair (guards.go:45)
    • Both runs are taken directly against the whitespace around =. A suffix/prefix comparison is therefore a true necessary condition for the regex.
    • The run alphabet is wider than the regex's first-character class, so the guard can only over-accept.
    • The k <= 51 cap equals 1 + {0,50}, which keeps the work per = constant. This fixes the quadratic behaviour.
    • EqualFold is safe here because the call site gates on isASCII.
  • hasTableFnCandidate (guards.go:95)
    • The regex needs whitespace directly before the name. Whitespace is not a word byte, so the backward word-run scan lands exactly on the name start.
    • The e-s < 2 and e-s > len(buf) bounds match s3 and azureblobstorage.
    • The chNames[string(buf[:n])] lookup does not allocate.
  • hasDollarOpener (dollar_quote.go): it reuses the lexer's own opener predicate (identifierByte and dollarTagEnd), so the two cannot drift apart.
  • ASCII gating: both (?i)-sensitive guards run only when isASCII holds, which covers the U+017F and U+212A folding concern. All guard use is also behind folded != nil, so other callers keep the old behaviour.
  • Earlier suggestions already addressed: chNames is now locked to the regex by TestTableFnGuardCoversEveryRegexName. The linearity test now measures wall-clock time, with a 3.5–3.7x ratio against a 10x limit.

Nits (non-blocking)

  1. Magic index. patternIndex == 2 (scanner.go:1217) breaks silently if the pattern list is reordered. Setting guard via withGuard on the compiled pattern, as the table-function pattern already does, would be sturdier.
  2. Redundant check. strings.IndexByte(sql, '$') < 0 || in stripDollarQuotedStrings is now covered by hasDollarOpener. It is harmless.
  3. isASCII cost. It is a full scan, recomputed per view for each guarded pattern. You already list this as a follow-up (W6-08/W6-10).
  4. chNames setup. The init() plus strings.Split is roundabout. A map literal would be simpler, and the exact-match test already prevents drift.
  5. Non-ASCII slow path. One non-ASCII byte takes the unguarded path. This matches today's behaviour, so it is not a regression, but a doc-comment note would help. Adversarial input can force this path cheaply.
  6. Timing tests under load. TestHasEqualIdentPairLinear is a wall-clock test and could be flaky on loaded CI runners. The margin is generous, and W0-04 already covers converting these tests to deterministic checks.

Performance and security

  • Findings are still regex-confirmed, so the only risk is a false negative. I found none in the logic, and the on/off equivalence test (corpora plus 200k seeded inputs) backs that up.
  • The amd64 CI evidence (about -63% geomean on ScanSQLRW) matches the M4 numbers. The perf-gate shadow run shows no regression on parse or tokenize.

Test coverage: strong. It has on/off equivalence, boundary tests at the 51/52-byte capture limit, a regex-name lockstep test, Unicode folding cases and fuzz seeds.
| Branch: perf/w6-01-scansql-guards

@ajitpratap0
ajitpratap0 merged commit 24879bd into main Nov 1, 2026
48 checks passed

This branch was successfully deployed

1 active deployment
Preview — 1175c2c1 Deployed Nov 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant