From 53bef8c6cc593e925afed8984912df0a97028c7d Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:27:55 +0530 Subject: [PATCH 1/4] perf(parser): size joins slice from a JOIN-count hint [W4-03] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 1 + pkg/sql/parser/join_hint_test.go | 126 +++++++++++++++++++++++++++++++ pkg/sql/parser/oracle_execute.go | 2 +- pkg/sql/parser/parser.go | 8 +- pkg/sql/parser/preprocess.go | 25 ++++-- pkg/sql/parser/select_clauses.go | 15 +++- 6 files changed, 168 insertions(+), 9 deletions(-) create mode 100644 pkg/sql/parser/join_hint_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a3666d73..66a7823f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 are removed. ### Performance +- **Parser joins slice sized from a JOIN-count hint [W4-03]**: the preprocess pass counts JOIN tokens and `parseJoinClauses` allocates its slice with capacity clamped to [1, 8] instead of a fixed 2. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. - **Linter**: `linter.Context` gains an additive `Nodes()` method returning every AST node in `ast.Inspect` preorder, built once per `Context` and cached (reset by `WithAST`). The 13 AST rules that each walked the tree with `ast.Inspect` now iterate that shared list; violations are unchanged (none of those walks pruned a subtree). [W6-05a] - **Security**: `Scanner.ScanSQL` now skips the dollar-quote stripping lexer, the identifier-tautology regex and the ClickHouse table-function regex on inputs that provably cannot match them, using linear-time necessary-condition guards (`hasDollarOpener`, `hasEqualIdentPair`, `hasTableFnCandidate`). The regexes still confirm every finding, so results are byte-identical (guarded by a guards-on/off equivalence test over the real-world corpus and 200k random inputs); no API or behaviour change. [W6-01] - **Tokenizer**: `time.Now`/`time.Since` are now read only when metrics are enabled (previously twice per tokenization even when disabled). If metrics are toggled on mid-call the call is not recorded. No API or token-output change [W1-02]. diff --git a/pkg/sql/parser/join_hint_test.go b/pkg/sql/parser/join_hint_test.go new file mode 100644 index 00000000..5593cda2 --- /dev/null +++ b/pkg/sql/parser/join_hint_test.go @@ -0,0 +1,126 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package parser + +import ( + "fmt" + "reflect" + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/models" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" +) + +func TestJoinsCap(t *testing.T) { + cases := map[int]int{-1: 2, 0: 2, 1: 1, 2: 2, 5: 5, 8: 8, 9: 8, 100: 8} + for hint, want := range cases { + p := &Parser{joinHint: hint} + if got := p.joinsCap(); got != want { + t.Errorf("joinHint=%d: cap=%d want %d", hint, got, want) + } + } +} + +func lexForHint(t *testing.T, sql string) []models.TokenWithSpan { + t.Helper() + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + toks, err := tkz.Tokenize([]byte(sql)) + if err != nil { + t.Fatalf("tokenize %q: %v", sql, err) + } + return append([]models.TokenWithSpan(nil), toks...) +} + +// The hint equals the number of JOIN tokens in the normalised stream and the +// token output is identical to preprocessTokensInto. +func TestPreprocessTokensCount(t *testing.T) { + cases := []struct { + sql string + want int + }{ + {"SELECT 1", 0}, + {"SELECT * FROM a JOIN b ON a.id = b.id", 1}, + {"SELECT * FROM a INNER JOIN b ON a.id = b.id LEFT JOIN c ON c.id = b.id", 2}, + {"SELECT * FROM a CROSS JOIN b", 1}, + {"SELECT * FROM a LEFT OUTER JOIN b ON a.x = b.x", 1}, + {"SELECT * FROM a JOIN (SELECT * FROM b JOIN c ON b.i = c.i) d ON a.i = d.i", 2}, + } + for _, c := range cases { + toks := lexForHint(t, c.sql) + got, n := preprocessTokensCount(nil, toks) + if n != c.want { + t.Errorf("%q: hint=%d want %d", c.sql, n, c.want) + } + if ref := preprocessTokensInto(nil, toks); !reflect.DeepEqual(got, ref) { + t.Errorf("%q: token stream differs from preprocessTokensInto", c.sql) + } + } +} + +// Parsing must not depend on the hint: every hint value yields an identical AST. +func TestJoinHintDoesNotChangeAST(t *testing.T) { + var parts []string + for i := 1; i <= 12; i++ { + parts = append(parts, fmt.Sprintf("JOIN t%d ON t%d.id = t0.id", i, i)) + } + queries := []string{ + "SELECT * FROM t0 " + strings.Join(parts[:1], " "), + "SELECT * FROM t0 " + strings.Join(parts[:3], " "), + "SELECT * FROM t0 " + strings.Join(parts, " "), + "SELECT * FROM t0 LEFT JOIN t1 ON t1.id = t0.id CROSS JOIN t2", + "SELECT * FROM (t0 JOIN t1 ON t1.id = t0.id) JOIN t2 ON t2.id = t0.id", + "SELECT * FROM t0, t1 JOIN t2 ON t2.id = t1.id", + } + for _, q := range queries { + toks := lexForHint(t, q) + var base *ast.AST + for _, hint := range []int{-1, 0, 1, 2, 3, 8, 50} { + p := &Parser{} + pre := preprocessTokensInto(nil, toks) + p.joinHint = hint + tree, err := p.parseTokens(pre) + if err != nil { + t.Fatalf("%q hint=%d: %v", q, hint, err) + } + if base == nil { + base = tree + continue + } + if !reflect.DeepEqual(base.Statements, tree.Statements) { + t.Errorf("%q: AST differs for hint=%d", q, hint) + } + } + } +} + +// Reset clears a stale hint, and preprocess refreshes it on every parse. +func TestJoinHintResetAndRefresh(t *testing.T) { + p := &Parser{} + p.preprocess(lexForHint(t, "SELECT * FROM a JOIN b ON a.i=b.i JOIN c ON c.i=b.i")) + if p.joinHint != 2 { + t.Fatalf("joinHint=%d want 2", p.joinHint) + } + p.Reset() + if p.joinHint != 0 { + t.Fatalf("Reset left joinHint=%d", p.joinHint) + } + p.preprocess(lexForHint(t, "SELECT 1")) + if p.joinHint != 0 { + t.Fatalf("joinHint=%d want 0 after join-free stream", p.joinHint) + } +} diff --git a/pkg/sql/parser/oracle_execute.go b/pkg/sql/parser/oracle_execute.go index 4fe459b7..fe317781 100644 --- a/pkg/sql/parser/oracle_execute.go +++ b/pkg/sql/parser/oracle_execute.go @@ -19,7 +19,7 @@ func (p *Parser) parseWithSource(tokens []models.TokenWithSpan, in sqlText) (*as return p.ParseFromModelTokens(tokens) } clear(p.tokenBuf) - p.tokenBuf = preprocessTokensInto(p.tokenBuf, tokens) + p.tokenBuf, p.joinHint = preprocessTokensCount(p.tokenBuf, tokens) p.oracleExecuteLines(in.source()) return p.parseTokens(p.tokenBuf) } diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 8dd72edc..237d6ae9 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -157,6 +157,7 @@ func (p *Parser) Reset() { clear(p.tokenBuf) p.tokenBuf = p.tokenBuf[:0] } + p.joinHint = 0 p.currentPos = 0 p.implicitEnd = 0 p.openBlockWork = 0 @@ -318,7 +319,10 @@ type Parser struct { // tokenBuf is parser-owned scratch holding the preprocessed token stream // (see preprocess). It is reused across parses and never exposed to // callers. INVARIANT: tokenBuf[len:cap] is always zeroed. - tokenBuf []models.TokenWithSpan + tokenBuf []models.TokenWithSpan + // joinHint is the JOIN token count of the preprocessed stream; it sizes + // the joins slice in parseJoinClauses (0 = unknown, use the default). + joinHint int currentPos int // currentToken points at p.tokens[currentPos], or at p.eof once the cursor // is past the stream. It is a pointer, not a copy, so advance moves 8 @@ -421,7 +425,7 @@ func (p *Parser) preprocess(tokens []models.TokenWithSpan) []models.TokenWithSpa // on the pooled path; on a parser reused without Reset it scrubs the // previous stream so a shorter one cannot leave stale strings in the tail. clear(p.tokenBuf) - p.tokenBuf = preprocessTokensInto(p.tokenBuf, tokens) + p.tokenBuf, p.joinHint = preprocessTokensCount(p.tokenBuf, tokens) p.oracleExecuteLines("") return p.tokenBuf } diff --git a/pkg/sql/parser/preprocess.go b/pkg/sql/parser/preprocess.go index fc369d34..9c61b8c1 100644 --- a/pkg/sql/parser/preprocess.go +++ b/pkg/sql/parser/preprocess.go @@ -59,28 +59,43 @@ func preprocessTokens(tokens []models.TokenWithSpan) []models.TokenWithSpan { // so it must not be handed to anyone who could outlive the next call with the // same dst. func preprocessTokensInto(dst, tokens []models.TokenWithSpan) []models.TokenWithSpan { + dst, _ = preprocessTokensCount(dst, tokens) + return dst +} + +// preprocessTokensCount is preprocessTokensInto that also returns the number +// of JOIN tokens in the normalised stream (compound INNER_JOIN etc. expand to +// [INNER, JOIN], so each counts once). The count only sizes the joins slice +// (Parser.joinHint); it is not a correctness input. +func preprocessTokensCount(dst, tokens []models.TokenWithSpan) ([]models.TokenWithSpan, int) { + joins := 0 dst = dst[:0] if need := len(tokens) + 4; cap(dst) < need { dst = make([]models.TokenWithSpan, 0, need) } for i := range tokens { t := &tokens[i] + before := len(dst) switch typ := t.Token.Type; { case typ == models.TokenTypeIdentifier || typ == models.TokenTypeKeyword || (typ >= models.TokenTypeCount && typ <= models.TokenTypeMax) || (typ >= models.TokenTypeGroupBy && typ <= models.TokenTypeCrossJoin) || typ == models.TokenTypeGroupingSets: var ok bool - if dst, ok = appendCompound(dst, t); ok { - continue + if dst, ok = appendCompound(dst, t); !ok { + dst = append(dst, *t) + normalizeTokenInPlace(&dst[len(dst)-1]) } - dst = append(dst, *t) - normalizeTokenInPlace(&dst[len(dst)-1]) default: dst = append(dst, *t) } + for k := before; k < len(dst); k++ { + if dst[k].Token.Type == models.TokenTypeJoin { + joins++ + } + } } - return dst + return dst, joins } // normalizeTokenInPlace remaps one token's type (and records a backtick quote) diff --git a/pkg/sql/parser/select_clauses.go b/pkg/sql/parser/select_clauses.go index 374d2a5e..3646bba0 100644 --- a/pkg/sql/parser/select_clauses.go +++ b/pkg/sql/parser/select_clauses.go @@ -410,7 +410,7 @@ func (p *Parser) parseJoinClauses(firstRef ast.TableReference, joins []ast.JoinC } if cap(joins) == 0 { - joins = make([]ast.JoinClause, 0, 2) + joins = make([]ast.JoinClause, 0, p.joinsCap()) } jc.Type = joinType jc.Left = leftTable @@ -1255,3 +1255,16 @@ func (p *Parser) isOrderByAll() bool { } return equalsUpper(p.peekToken().Token.Value, "NULLS") } + +// joinsCap returns the initial capacity for a statement's joins slice: the +// JOIN token count of the stream clamped to [1, 8], or 2 when no hint exists. +func (p *Parser) joinsCap() int { + switch h := p.joinHint; { + case h <= 0: + return 2 + case h > 8: + return 8 + default: + return h + } +} From b0eeb75f5d480da495db10a3b6f13af58d3f0c6b Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:54:05 +0530 Subject: [PATCH 2/4] perf(parser): count JOINs lazily at allocation instead of in preprocess [W4-03] The preprocess-loop counter cost about 1.3% instructions on join-free queries (CI perf gate). Scan a 256-token lookahead only when the first join is appended, so join-free statements pay nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 2 +- pkg/sql/parser/join_hint_test.go | 146 +++++++++++++++---------------- pkg/sql/parser/oracle_execute.go | 2 +- pkg/sql/parser/parser.go | 8 +- pkg/sql/parser/preprocess.go | 25 ++---- pkg/sql/parser/select_clauses.go | 23 +++-- 6 files changed, 94 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66a7823f..01e74498 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 are removed. ### Performance -- **Parser joins slice sized from a JOIN-count hint [W4-03]**: the preprocess pass counts JOIN tokens and `parseJoinClauses` allocates its slice with capacity clamped to [1, 8] instead of a fixed 2. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. +- **Parser joins slice sized from a JOIN lookahead [W4-03]**: when the first JOIN of a FROM clause is appended, `parseJoinClauses` counts JOIN tokens in the next 256 tokens and allocates the slice with capacity 1 + that count (max 8) instead of a fixed 2. Join-free statements pay nothing. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. - **Linter**: `linter.Context` gains an additive `Nodes()` method returning every AST node in `ast.Inspect` preorder, built once per `Context` and cached (reset by `WithAST`). The 13 AST rules that each walked the tree with `ast.Inspect` now iterate that shared list; violations are unchanged (none of those walks pruned a subtree). [W6-05a] - **Security**: `Scanner.ScanSQL` now skips the dollar-quote stripping lexer, the identifier-tautology regex and the ClickHouse table-function regex on inputs that provably cannot match them, using linear-time necessary-condition guards (`hasDollarOpener`, `hasEqualIdentPair`, `hasTableFnCandidate`). The regexes still confirm every finding, so results are byte-identical (guarded by a guards-on/off equivalence test over the real-world corpus and 200k random inputs); no API or behaviour change. [W6-01] - **Tokenizer**: `time.Now`/`time.Since` are now read only when metrics are enabled (previously twice per tokenization even when disabled). If metrics are toggled on mid-call the call is not recorded. No API or token-output change [W1-02]. diff --git a/pkg/sql/parser/join_hint_test.go b/pkg/sql/parser/join_hint_test.go index 5593cda2..a4d6b3c6 100644 --- a/pkg/sql/parser/join_hint_test.go +++ b/pkg/sql/parser/join_hint_test.go @@ -16,7 +16,6 @@ package parser import ( "fmt" - "reflect" "strings" "testing" @@ -25,17 +24,7 @@ import ( "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" ) -func TestJoinsCap(t *testing.T) { - cases := map[int]int{-1: 2, 0: 2, 1: 1, 2: 2, 5: 5, 8: 8, 9: 8, 100: 8} - for hint, want := range cases { - p := &Parser{joinHint: hint} - if got := p.joinsCap(); got != want { - t.Errorf("joinHint=%d: cap=%d want %d", hint, got, want) - } - } -} - -func lexForHint(t *testing.T, sql string) []models.TokenWithSpan { +func joinHintTokens(t *testing.T, sql string) []models.TokenWithSpan { t.Helper() tkz := tokenizer.GetTokenizer() defer tokenizer.PutTokenizer(tkz) @@ -43,84 +32,89 @@ func lexForHint(t *testing.T, sql string) []models.TokenWithSpan { if err != nil { t.Fatalf("tokenize %q: %v", sql, err) } - return append([]models.TokenWithSpan(nil), toks...) + return preprocessTokens(toks) } -// The hint equals the number of JOIN tokens in the normalised stream and the -// token output is identical to preprocessTokensInto. -func TestPreprocessTokensCount(t *testing.T) { - cases := []struct { - sql string - want int - }{ - {"SELECT 1", 0}, - {"SELECT * FROM a JOIN b ON a.id = b.id", 1}, - {"SELECT * FROM a INNER JOIN b ON a.id = b.id LEFT JOIN c ON c.id = b.id", 2}, - {"SELECT * FROM a CROSS JOIN b", 1}, - {"SELECT * FROM a LEFT OUTER JOIN b ON a.x = b.x", 1}, - {"SELECT * FROM a JOIN (SELECT * FROM b JOIN c ON b.i = c.i) d ON a.i = d.i", 2}, +func joinsOf(t *testing.T, sql string) []ast.JoinClause { + t.Helper() + p := &Parser{} + tree, err := p.parseTokens(joinHintTokens(t, sql)) + if err != nil { + t.Fatalf("parse %q: %v", sql, err) } - for _, c := range cases { - toks := lexForHint(t, c.sql) - got, n := preprocessTokensCount(nil, toks) - if n != c.want { - t.Errorf("%q: hint=%d want %d", c.sql, n, c.want) - } - if ref := preprocessTokensInto(nil, toks); !reflect.DeepEqual(got, ref) { - t.Errorf("%q: token stream differs from preprocessTokensInto", c.sql) - } + sel, ok := tree.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("expected SelectStatement, got %T", tree.Statements[0]) } + return sel.Joins } -// Parsing must not depend on the hint: every hint value yields an identical AST. -func TestJoinHintDoesNotChangeAST(t *testing.T) { - var parts []string - for i := 1; i <= 12; i++ { - parts = append(parts, fmt.Sprintf("JOIN t%d ON t%d.id = t0.id", i, i)) - } - queries := []string{ - "SELECT * FROM t0 " + strings.Join(parts[:1], " "), - "SELECT * FROM t0 " + strings.Join(parts[:3], " "), - "SELECT * FROM t0 " + strings.Join(parts, " "), - "SELECT * FROM t0 LEFT JOIN t1 ON t1.id = t0.id CROSS JOIN t2", - "SELECT * FROM (t0 JOIN t1 ON t1.id = t0.id) JOIN t2 ON t2.id = t0.id", - "SELECT * FROM t0, t1 JOIN t2 ON t2.id = t1.id", +func joinQuery(n int) string { + var b strings.Builder + b.WriteString("SELECT * FROM t0") + for i := 1; i <= n; i++ { + fmt.Fprintf(&b, " JOIN t%d ON t%d.id = t0.id", i, i) } - for _, q := range queries { - toks := lexForHint(t, q) - var base *ast.AST - for _, hint := range []int{-1, 0, 1, 2, 3, 8, 50} { - p := &Parser{} - pre := preprocessTokensInto(nil, toks) - p.joinHint = hint - tree, err := p.parseTokens(pre) - if err != nil { - t.Fatalf("%q hint=%d: %v", q, hint, err) - } - if base == nil { - base = tree - continue - } - if !reflect.DeepEqual(base.Statements, tree.Statements) { - t.Errorf("%q: AST differs for hint=%d", q, hint) + return b.String() +} + +// The initial joins capacity is 1 plus the JOINs ahead, capped at 8, and the +// parsed joins are complete and ordered for every count, including above the cap. +func TestJoinsSliceCapacityAndContent(t *testing.T) { + for _, n := range []int{1, 2, 3, 5, 8, 9, 12} { + joins := joinsOf(t, joinQuery(n)) + if len(joins) != n { + t.Fatalf("n=%d: got %d joins", n, len(joins)) + } + for i, j := range joins { + if want := fmt.Sprintf("t%d", i+1); j.Right.Name != want { + t.Errorf("n=%d join %d: right=%q want %q", n, i, j.Right.Name, want) } } + wantCap := min(n, 8) + if n <= 8 && cap(joins) != wantCap { + t.Errorf("n=%d: cap=%d want %d", n, cap(joins), wantCap) + } + if n > 8 && cap(joins) < n { + t.Errorf("n=%d: cap=%d below len", n, cap(joins)) + } + } +} + +func TestJoinsCapWindowAndCap(t *testing.T) { + p := &Parser{} + p.tokens = joinHintTokens(t, joinQuery(20)) + p.currentPos = 0 + if got := p.joinsCap(); got != 8 { + t.Errorf("cap=%d want 8", got) + } + // Past the end of the stream there is nothing to look at. + p.currentPos = len(p.tokens) + if got := p.joinsCap(); got != 1 { + t.Errorf("cap=%d want 1 at end of stream", got) } } -// Reset clears a stale hint, and preprocess refreshes it on every parse. -func TestJoinHintResetAndRefresh(t *testing.T) { +// A join-free statement among join statements must not be affected, and a +// multi-statement script parses identically statement by statement. +func TestJoinsCapMultiStatement(t *testing.T) { + script := "SELECT * FROM a JOIN b ON a.i=b.i; SELECT 1; SELECT * FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i" p := &Parser{} - p.preprocess(lexForHint(t, "SELECT * FROM a JOIN b ON a.i=b.i JOIN c ON c.i=b.i")) - if p.joinHint != 2 { - t.Fatalf("joinHint=%d want 2", p.joinHint) + tree, err := p.parseTokens(joinHintTokens(t, script)) + if err != nil { + t.Fatal(err) } - p.Reset() - if p.joinHint != 0 { - t.Fatalf("Reset left joinHint=%d", p.joinHint) + want := []int{1, 0, 2} + if len(tree.Statements) != len(want) { + t.Fatalf("got %d statements", len(tree.Statements)) } - p.preprocess(lexForHint(t, "SELECT 1")) - if p.joinHint != 0 { - t.Fatalf("joinHint=%d want 0 after join-free stream", p.joinHint) + for i, st := range tree.Statements { + sel, ok := st.(*ast.SelectStatement) + if !ok { + t.Fatalf("stmt %d: %T", i, st) + } + if len(sel.Joins) != want[i] { + t.Errorf("stmt %d: %d joins want %d", i, len(sel.Joins), want[i]) + } } } diff --git a/pkg/sql/parser/oracle_execute.go b/pkg/sql/parser/oracle_execute.go index fe317781..4fe459b7 100644 --- a/pkg/sql/parser/oracle_execute.go +++ b/pkg/sql/parser/oracle_execute.go @@ -19,7 +19,7 @@ func (p *Parser) parseWithSource(tokens []models.TokenWithSpan, in sqlText) (*as return p.ParseFromModelTokens(tokens) } clear(p.tokenBuf) - p.tokenBuf, p.joinHint = preprocessTokensCount(p.tokenBuf, tokens) + p.tokenBuf = preprocessTokensInto(p.tokenBuf, tokens) p.oracleExecuteLines(in.source()) return p.parseTokens(p.tokenBuf) } diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 237d6ae9..8dd72edc 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -157,7 +157,6 @@ func (p *Parser) Reset() { clear(p.tokenBuf) p.tokenBuf = p.tokenBuf[:0] } - p.joinHint = 0 p.currentPos = 0 p.implicitEnd = 0 p.openBlockWork = 0 @@ -319,10 +318,7 @@ type Parser struct { // tokenBuf is parser-owned scratch holding the preprocessed token stream // (see preprocess). It is reused across parses and never exposed to // callers. INVARIANT: tokenBuf[len:cap] is always zeroed. - tokenBuf []models.TokenWithSpan - // joinHint is the JOIN token count of the preprocessed stream; it sizes - // the joins slice in parseJoinClauses (0 = unknown, use the default). - joinHint int + tokenBuf []models.TokenWithSpan currentPos int // currentToken points at p.tokens[currentPos], or at p.eof once the cursor // is past the stream. It is a pointer, not a copy, so advance moves 8 @@ -425,7 +421,7 @@ func (p *Parser) preprocess(tokens []models.TokenWithSpan) []models.TokenWithSpa // on the pooled path; on a parser reused without Reset it scrubs the // previous stream so a shorter one cannot leave stale strings in the tail. clear(p.tokenBuf) - p.tokenBuf, p.joinHint = preprocessTokensCount(p.tokenBuf, tokens) + p.tokenBuf = preprocessTokensInto(p.tokenBuf, tokens) p.oracleExecuteLines("") return p.tokenBuf } diff --git a/pkg/sql/parser/preprocess.go b/pkg/sql/parser/preprocess.go index 9c61b8c1..fc369d34 100644 --- a/pkg/sql/parser/preprocess.go +++ b/pkg/sql/parser/preprocess.go @@ -59,43 +59,28 @@ func preprocessTokens(tokens []models.TokenWithSpan) []models.TokenWithSpan { // so it must not be handed to anyone who could outlive the next call with the // same dst. func preprocessTokensInto(dst, tokens []models.TokenWithSpan) []models.TokenWithSpan { - dst, _ = preprocessTokensCount(dst, tokens) - return dst -} - -// preprocessTokensCount is preprocessTokensInto that also returns the number -// of JOIN tokens in the normalised stream (compound INNER_JOIN etc. expand to -// [INNER, JOIN], so each counts once). The count only sizes the joins slice -// (Parser.joinHint); it is not a correctness input. -func preprocessTokensCount(dst, tokens []models.TokenWithSpan) ([]models.TokenWithSpan, int) { - joins := 0 dst = dst[:0] if need := len(tokens) + 4; cap(dst) < need { dst = make([]models.TokenWithSpan, 0, need) } for i := range tokens { t := &tokens[i] - before := len(dst) switch typ := t.Token.Type; { case typ == models.TokenTypeIdentifier || typ == models.TokenTypeKeyword || (typ >= models.TokenTypeCount && typ <= models.TokenTypeMax) || (typ >= models.TokenTypeGroupBy && typ <= models.TokenTypeCrossJoin) || typ == models.TokenTypeGroupingSets: var ok bool - if dst, ok = appendCompound(dst, t); !ok { - dst = append(dst, *t) - normalizeTokenInPlace(&dst[len(dst)-1]) + if dst, ok = appendCompound(dst, t); ok { + continue } + dst = append(dst, *t) + normalizeTokenInPlace(&dst[len(dst)-1]) default: dst = append(dst, *t) } - for k := before; k < len(dst); k++ { - if dst[k].Token.Type == models.TokenTypeJoin { - joins++ - } - } } - return dst, joins + return dst } // normalizeTokenInPlace remaps one token's type (and records a backtick quote) diff --git a/pkg/sql/parser/select_clauses.go b/pkg/sql/parser/select_clauses.go index 3646bba0..6e0c7b9e 100644 --- a/pkg/sql/parser/select_clauses.go +++ b/pkg/sql/parser/select_clauses.go @@ -1256,15 +1256,22 @@ func (p *Parser) isOrderByAll() bool { return equalsUpper(p.peekToken().Token.Value, "NULLS") } +// joinHintWindow bounds how far joinsCap looks ahead for JOIN tokens, so the +// scan stays O(1) per statement on huge scripts. +const joinHintWindow = 256 + // joinsCap returns the initial capacity for a statement's joins slice: the -// JOIN token count of the stream clamped to [1, 8], or 2 when no hint exists. +// 1 plus the number of JOIN tokens in the next joinHintWindow tokens, capped at 8. +// It runs only when a JOIN is about to be appended to an empty slice, so +// join-free statements pay nothing. The count is a capacity hint only (it may +// include joins of nested subqueries or of the next statement). func (p *Parser) joinsCap() int { - switch h := p.joinHint; { - case h <= 0: - return 2 - case h > 8: - return 8 - default: - return h + end := min(p.currentPos+joinHintWindow, len(p.tokens)) + n := 1 // the join being appended; its JOIN token is already consumed + for i := p.currentPos; i < end && n < 8; i++ { + if p.tokens[i].Token.Type == models.TokenTypeJoin { + n++ + } } + return n } From 962eff045395fd34b447127a3cbe23c3b1f01ad6 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 20:22:21 +0530 Subject: [PATCH 3/4] perf(parser): bound joinsCap by paren depth and statement/set-op boundaries [W4-03] The 256-token lookahead counted JOINs of neighbouring statements, UNION branches and nested subqueries, so short single-join statements in scripts allocated cap 8 (416 B per JoinClause), doubling B/op. Count only depth-0 JOINs and stop at ';', UNION/EXCEPT/INTERSECT or the closing ')'. Adds capacity assertions for scripts, set-op branches and subquery depth, and a many-short-join-statements benchmark. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 2 +- pkg/sql/parser/join_hint_test.go | 117 +++++++++++++++++++++++++++++++ pkg/sql/parser/select_clauses.go | 33 ++++++--- 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01e74498..1e7b74f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 are removed. ### Performance -- **Parser joins slice sized from a JOIN lookahead [W4-03]**: when the first JOIN of a FROM clause is appended, `parseJoinClauses` counts JOIN tokens in the next 256 tokens and allocates the slice with capacity 1 + that count (max 8) instead of a fixed 2. Join-free statements pay nothing. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. +- **Parser joins slice sized from a JOIN lookahead [W4-03]**: when the first JOIN of a FROM clause is appended, `parseJoinClauses` counts the JOIN tokens that follow at the same paren depth (within 256 tokens, stopping at `;`, UNION/EXCEPT/INTERSECT or the closing `)`) and allocates the slice with capacity 1 + that count (max 8) instead of a fixed 2. Join-free statements pay nothing. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. - **Linter**: `linter.Context` gains an additive `Nodes()` method returning every AST node in `ast.Inspect` preorder, built once per `Context` and cached (reset by `WithAST`). The 13 AST rules that each walked the tree with `ast.Inspect` now iterate that shared list; violations are unchanged (none of those walks pruned a subtree). [W6-05a] - **Security**: `Scanner.ScanSQL` now skips the dollar-quote stripping lexer, the identifier-tautology regex and the ClickHouse table-function regex on inputs that provably cannot match them, using linear-time necessary-condition guards (`hasDollarOpener`, `hasEqualIdentPair`, `hasTableFnCandidate`). The regexes still confirm every finding, so results are byte-identical (guarded by a guards-on/off equivalence test over the real-world corpus and 200k random inputs); no API or behaviour change. [W6-01] - **Tokenizer**: `time.Now`/`time.Since` are now read only when metrics are enabled (previously twice per tokenization even when disabled). If metrics are toggled on mid-call the call is not recorded. No API or token-output change [W1-02]. diff --git a/pkg/sql/parser/join_hint_test.go b/pkg/sql/parser/join_hint_test.go index a4d6b3c6..c303a511 100644 --- a/pkg/sql/parser/join_hint_test.go +++ b/pkg/sql/parser/join_hint_test.go @@ -118,3 +118,120 @@ func TestJoinsCapMultiStatement(t *testing.T) { } } } + +func selectStatements(t *testing.T, sql string) []*ast.SelectStatement { + t.Helper() + p := &Parser{} + tree, err := p.parseTokens(joinHintTokens(t, sql)) + if err != nil { + t.Fatalf("parse %q: %v", sql, err) + } + var out []*ast.SelectStatement + var walk func(st ast.Statement) + walk = func(st ast.Statement) { + switch s := st.(type) { + case *ast.SelectStatement: + out = append(out, s) + for _, f := range s.From { + if f.Subquery != nil { + walk(f.Subquery) + } + } + case *ast.SetOperation: + walk(s.Left) + walk(s.Right) + } + } + for _, st := range tree.Statements { + walk(st) + } + return out +} + +// The hint must not look past the current statement, set-operation branch or +// subquery: every one-join SELECT below must get capacity exactly 1. +func TestJoinsCapStopsAtBoundaries(t *testing.T) { + cases := []struct { + name string + sql string + want []int // cap of Joins for each SELECT with joins, in order + }{ + {"script of single-join statements", + strings.Repeat("SELECT * FROM a JOIN b ON a.i=b.i; ", 5), []int{1, 1, 1, 1, 1}}, + {"union branches", + "SELECT a.x FROM a JOIN b ON a.i=b.i UNION ALL SELECT c.x FROM c JOIN d ON c.i=d.i", []int{1, 1}}, + {"except branches", + "SELECT a.x FROM a JOIN b ON a.i=b.i EXCEPT SELECT c.x FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i", []int{1, 2}}, + {"intersect branches", + "SELECT a.x FROM a JOIN b ON a.i=b.i INTERSECT SELECT c.x FROM c JOIN d ON c.i=d.i", []int{1, 1}}, + {"mixed one and two join statements", + "SELECT * FROM a JOIN b ON a.i=b.i; SELECT * FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i; SELECT * FROM f JOIN g ON f.i=g.i", []int{1, 2, 1}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var got []int + for _, s := range selectStatements(t, tc.sql) { + if len(s.Joins) > 0 { + got = append(got, cap(s.Joins)) + } + } + if fmt.Sprint(got) != fmt.Sprint(tc.want) { + t.Errorf("caps=%v want %v", got, tc.want) + } + }) + } +} + +// A subquery's join must not count the outer query's later joins, and the +// outer query must not count joins nested in subqueries. +func TestJoinsCapSubqueryDepth(t *testing.T) { + sql := "SELECT * FROM (SELECT * FROM a JOIN b ON a.i=b.i) s JOIN c ON c.i=s.i JOIN d ON d.i=c.i" + var inner, outer *ast.SelectStatement + for _, s := range selectStatements(t, sql) { + if len(s.Joins) == 2 { + outer = s + } else if len(s.Joins) == 1 { + inner = s + } + } + if inner == nil || outer == nil { + t.Fatalf("did not find inner and outer selects (inner=%v outer=%v)", inner != nil, outer != nil) + } + if cap(inner.Joins) != 1 { + t.Errorf("subquery joins cap=%d want 1", cap(inner.Joins)) + } + if cap(outer.Joins) != 2 { + t.Errorf("outer joins cap=%d want 2", cap(outer.Joins)) + } + + // Joins nested inside a parenthesised subquery ahead do not count. + sql = "SELECT * FROM a JOIN b ON b.i IN (SELECT x FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i)" + for _, s := range selectStatements(t, sql) { + if len(s.Joins) == 1 && cap(s.Joins) != 1 { + t.Errorf("outer cap=%d want 1", cap(s.Joins)) + } + } +} + +// BenchmarkParseManyShortJoinStatements keeps the multi-statement trade-off +// visible: each statement has one JOIN, so each joins slice must stay cap 1. +func BenchmarkParseManyShortJoinStatements(b *testing.B) { + sql := strings.Repeat("SELECT * FROM a JOIN b ON a.i=b.i; ", 50) + tkz := tokenizer.GetTokenizer() + toks, err := tkz.Tokenize([]byte(sql)) + if err != nil { + b.Fatal(err) + } + pt := preprocessTokens(toks) + tokenizer.PutTokenizer(tkz) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + p := &Parser{} + tree, err := p.parseTokens(pt) + if err != nil { + b.Fatal(err) + } + ast.ReleaseAST(tree) + } +} diff --git a/pkg/sql/parser/select_clauses.go b/pkg/sql/parser/select_clauses.go index 6e0c7b9e..ef501828 100644 --- a/pkg/sql/parser/select_clauses.go +++ b/pkg/sql/parser/select_clauses.go @@ -1260,17 +1260,34 @@ func (p *Parser) isOrderByAll() bool { // scan stays O(1) per statement on huge scripts. const joinHintWindow = 256 -// joinsCap returns the initial capacity for a statement's joins slice: the -// 1 plus the number of JOIN tokens in the next joinHintWindow tokens, capped at 8. -// It runs only when a JOIN is about to be appended to an empty slice, so -// join-free statements pay nothing. The count is a capacity hint only (it may -// include joins of nested subqueries or of the next statement). +// joinsCap returns the initial capacity for a statement's joins slice: 1 plus +// the number of JOIN tokens that follow at the same parenthesis depth, capped +// at 8. The scan stops at a depth-0 ';', UNION, EXCEPT or INTERSECT, or at the +// ')' closing the enclosing level, so neighbouring statements, set-operation +// branches and nested subqueries never inflate the hint. It also looks at most +// joinHintWindow tokens ahead. It runs only when a JOIN is about to be +// appended to an empty slice, so join-free statements pay nothing. The result +// is a capacity hint only; append still grows past it. func (p *Parser) joinsCap() int { end := min(p.currentPos+joinHintWindow, len(p.tokens)) - n := 1 // the join being appended; its JOIN token is already consumed + n, depth := 1, 0 // n counts the join being appended; its JOIN token is already consumed for i := p.currentPos; i < end && n < 8; i++ { - if p.tokens[i].Token.Type == models.TokenTypeJoin { - n++ + switch p.tokens[i].Token.Type { + case models.TokenTypeJoin: + if depth == 0 { + n++ + } + case models.TokenTypeLParen: + depth++ + case models.TokenTypeRParen: + if depth == 0 { + return n + } + depth-- + case models.TokenTypeSemicolon, models.TokenTypeUnion, models.TokenTypeExcept, models.TokenTypeIntersect: + if depth == 0 { + return n + } } } return n From f82e15c1f421740ed6137426252a45ab1868e961 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Mon, 2 Nov 2026 20:44:46 +0530 Subject: [PATCH 4/4] perf(parser): grow joins by hint when full; fix subquery-depth test [W4-03] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 2 +- pkg/sql/parser/join_hint_test.go | 57 +++++++++++++++++++++++++++++--- pkg/sql/parser/select_clauses.go | 12 +++++-- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae2babf4..acbb2d2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 are removed. ### Performance -- **Parser joins slice sized from a JOIN lookahead [W4-03]**: when the first JOIN of a FROM clause is appended, `parseJoinClauses` counts the JOIN tokens that follow at the same paren depth (within 256 tokens, stopping at `;`, UNION/EXCEPT/INTERSECT or the closing `)`) and allocates the slice with capacity 1 + that count (max 8) instead of a fixed 2. Join-free statements pay nothing. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. +- **Parser joins slice sized from a JOIN lookahead [W4-03]**: when the first JOIN of a FROM clause is appended, `parseJoinClauses` counts the JOIN tokens that follow at the same paren depth (within 256 tokens, stopping at `;`, UNION/EXCEPT/INTERSECT or the closing `)`) and allocates the slice with capacity 1 + that count (max 8) instead of a fixed 2. Join-free statements pay nothing. `ParseE2E/join` and `complex` B/op drop about 20%; no API or AST change. A full slice handed in by a parenthesized join group or a comma table is grown by the same hint. Known edge cost: shapes the lookahead cannot see (`(a JOIN b) JOIN c`, `a JOIN b, c`) incur one extra allocation per statement; the gated suite is unaffected. - **Linter**: `linter.Context` gains an additive `Nodes()` method returning every AST node in `ast.Inspect` preorder, built once per `Context` and cached (reset by `WithAST`). The 13 AST rules that each walked the tree with `ast.Inspect` now iterate that shared list; violations are unchanged (none of those walks pruned a subtree). [W6-05a] - **Security**: `Scanner.ScanSQL` now skips the dollar-quote stripping lexer, the identifier-tautology regex and the ClickHouse table-function regex on inputs that provably cannot match them, using linear-time necessary-condition guards (`hasDollarOpener`, `hasEqualIdentPair`, `hasTableFnCandidate`). The regexes still confirm every finding, so results are byte-identical (guarded by a guards-on/off equivalence test over the real-world corpus and 200k random inputs); no API or behaviour change. [W6-01] - **Tokenizer**: `time.Now`/`time.Since` are now read only when metrics are enabled (previously twice per tokenization even when disabled). If metrics are toggled on mid-call the call is not recorded. No API or token-output change [W1-02]. diff --git a/pkg/sql/parser/join_hint_test.go b/pkg/sql/parser/join_hint_test.go index c303a511..574e2016 100644 --- a/pkg/sql/parser/join_hint_test.go +++ b/pkg/sql/parser/join_hint_test.go @@ -204,15 +204,62 @@ func TestJoinsCapSubqueryDepth(t *testing.T) { t.Errorf("outer joins cap=%d want 2", cap(outer.Joins)) } - // Joins nested inside a parenthesised subquery ahead do not count. - sql = "SELECT * FROM a JOIN b ON b.i IN (SELECT x FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i)" - for _, s := range selectStatements(t, sql) { - if len(s.Joins) == 1 && cap(s.Joins) != 1 { - t.Errorf("outer cap=%d want 1", cap(s.Joins)) + // Joins nested in a subquery that sits after the first join (WHERE, or a + // derived table in a later join) must not count toward the outer cap. + for _, tc := range []struct { + sql string + want int + }{ + {"SELECT * FROM a JOIN b ON a.i=b.i WHERE a.x IN (SELECT x FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i)", 1}, + {"SELECT * FROM a JOIN b ON a.i=b.i JOIN (SELECT * FROM c JOIN d ON c.i=d.i JOIN e ON e.i=d.i) s ON s.i=a.i", 2}, + } { + found := false + for _, s := range selectStatements(t, tc.sql) { + if s.Joins != nil && s.Joins[0].Left.Name == "a" { + found = true + if cap(s.Joins) != tc.want { + t.Errorf("%s: outer cap=%d want %d", tc.sql, cap(s.Joins), tc.want) + } + } + } + if !found { + t.Errorf("%s: outer select not found", tc.sql) } } } +// A parenthesized join group hands its slice to the outer join loop; the outer +// joins must grow it by the hint rather than reallocating, so the capacity is +// reached in at most one extra growth and never exceeds the join count rounded +// to the hint. +func TestJoinsCapParenGroupThenOuter(t *testing.T) { + for _, tc := range []struct { + sql string + joins int + maxCap int + }{ + {"SELECT * FROM (a JOIN b ON a.i=b.i) JOIN c ON c.i=a.i", 2, 3}, + {"SELECT * FROM (a JOIN b ON a.i=b.i) JOIN c ON c.i=a.i JOIN d ON d.i=a.i", 3, 4}, + } { + got := joinsOf(t, tc.sql) + if len(got) != tc.joins { + t.Fatalf("%s: joins=%d want %d", tc.sql, len(got), tc.joins) + } + if cap(got) > tc.maxCap { + t.Errorf("%s: cap=%d want <= %d", tc.sql, cap(got), tc.maxCap) + } + } +} + +// Known trade-off: a comma table after a single JOIN appends to the cap-1 +// slice, costing one growth. Content must still be correct. +func TestJoinsCommaAfterJoin(t *testing.T) { + got := joinsOf(t, "SELECT * FROM a JOIN b ON a.i=b.i, c") + if len(got) != 2 || !got[1].Comma || got[1].Type != "CROSS" { + t.Fatalf("unexpected joins: %+v", got) + } +} + // BenchmarkParseManyShortJoinStatements keeps the multi-statement trade-off // visible: each statement has one JOIN, so each joins slice must stay cap 1. func BenchmarkParseManyShortJoinStatements(b *testing.B) { diff --git a/pkg/sql/parser/select_clauses.go b/pkg/sql/parser/select_clauses.go index ef501828..531cabe8 100644 --- a/pkg/sql/parser/select_clauses.go +++ b/pkg/sql/parser/select_clauses.go @@ -20,6 +20,7 @@ package parser import ( "fmt" + "slices" "strconv" "strings" @@ -409,8 +410,11 @@ func (p *Parser) parseJoinClauses(firstRef ast.TableReference, joins []ast.JoinC } } - if cap(joins) == 0 { - joins = make([]ast.JoinClause, 0, p.joinsCap()) + if len(joins) == cap(joins) { + // Empty slice: size it from the JOINs ahead. Full slice handed in + // by a caller (parenthesized group, comma join): grow by the hint + // once instead of letting append double repeatedly. + joins = slices.Grow(joins, p.joinsCap()) } jc.Type = joinType jc.Left = leftTable @@ -1267,7 +1271,9 @@ const joinHintWindow = 256 // branches and nested subqueries never inflate the hint. It also looks at most // joinHintWindow tokens ahead. It runs only when a JOIN is about to be // appended to an empty slice, so join-free statements pay nothing. The result -// is a capacity hint only; append still grows past it. +// is a capacity hint only; append still grows past it. Oracle MINUS is +// tokenized as an identifier, so it is not a boundary and a MINUS chain may +// over-count later branches' JOINs (bounded by the cap and window). func (p *Parser) joinsCap() int { end := min(p.currentPos+joinHintWindow, len(p.tokens)) n, depth := 1, 0 // n counts the join being appended; its JOIN token is already consumed