perf(security): pooled struct visitor for Scanner.Scan [W6-02]

CHANGELOG.md

@@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     benchmarks that publish diagnostics no longer time a growing buffer.
 
 ### Performance
+- Security: `Scanner.Scan` walks the AST with a pooled struct visitor instead of a per-call closure and ancestor slice; `BenchmarkScanASTRW` B/op −53.27% to −58.31%, allocs/op −29% to −33% (M4, indicative). Findings are byte-identical (differential test against the previous implementation over the corpus); no API or behaviour change. [W6-02]
 - **Linter**: rule L007 (keyword case consistency) scans words without allocating (substring words, ASCII upper-casing into a stack buffer before the keyword lookup). Output is unchanged. On the canonical benchmarks (Apple M4, local), `BenchmarkLintRule/L007` allocates about 98% fewer objects (72565 to 1194 allocs/op) and about 97% fewer bytes, and `BenchmarkLintRW` allocates about 12.5% fewer objects. [W6-03]
 - **gosqlx**: errors returned by `Parse`, `Validate`, `Lint` and the tree helpers now wrap the sentinel and the underlying error in an unexported `wrapErr` that builds its message lazily, instead of `fmt.Errorf("%w: %w", ...)` on every failure. Error text, `errors.Is` and `errors.As` behaviour are unchanged; only the unexported concrete error type differs. The saving applies to callers that only check `errors.Is`/`errors.As` or drop the message: when the message is built (`err.Error()`, as the LSP and CLI do) the gain is about 2 allocs/op and 3-4% B/op per error (syntax_mid 21 -> 19 allocs). Because the message is now built on demand, `Error()` reflects any later mutation of the inner `*errors.Error` reached through `errors.As`; previously the text was fixed at creation. [W1-03]
 - **Linter L010**: scans bytes in a single pass instead of rebuilding

pkg/sql/security/scan_walker.go

@@ -0,0 +1,133 @@
+package security
+
+import (
+	"sync"
+
+	"github.com/ajitpratap0/GoSQLX/pkg/sql/ast"
+)
+
+const maxPooledScanDepth = 1024
+
+// scanWalker is exclusively owned between acquire and release. Between scans,
+// len(ancestors) == 0 and every slot in ancestors[:cap(ancestors)] is nil.
+type scanWalker struct {
+	s         *Scanner
+	result    *ScanResult
+	ancestors []ast.Node
+}
+
+var scanWalkerPool = sync.Pool{New: func() interface{} {
+	return &scanWalker{ancestors: make([]ast.Node, 0, 16)}
+}}
+
+func (w *scanWalker) reset(s *Scanner, r *ScanResult) {
+	w.s, w.result = s, r
+	w.ancestors = w.ancestors[:0]
+}
+
+func acquireScanWalker(s *Scanner, r *ScanResult) *scanWalker {
+	w, ok := scanWalkerPool.Get().(*scanWalker)
+	if !ok {
+		w = &scanWalker{ancestors: make([]ast.Node, 0, 16)}
+	}
+	w.reset(s, r)
+	return w
+}
+
+func (w *scanWalker) release() (pooled bool) {
+	w.s, w.result = nil, nil
+	if cap(w.ancestors) > maxPooledScanDepth {
+		return false
+	}
+	w.ancestors = w.ancestors[:0]
+	scanWalkerPool.Put(w)
+	return true
+}
+
+func (w *scanWalker) Visit(node ast.Node) (ast.Visitor, error) {
+	if w.visit(node) {
+		return w, nil
+	}
+	return nil, nil
+}
+
+func (w *scanWalker) visit(node ast.Node) bool {
+	if node == nil {
+		w.ancestors[len(w.ancestors)-1] = nil
+		w.ancestors = w.ancestors[:len(w.ancestors)-1]
+		return false
+	}
+	var parent ast.Node
+	if len(w.ancestors) > 0 {
+		parent = w.ancestors[len(w.ancestors)-1]
+	}
+	if join, ok := parent.(*ast.JoinClause); ok && node == &join.Left {
+		// Left repeats FROM or an earlier join's right reference,
+		// including its subquery and table function.
+		return false
+	}
+	switch n := node.(type) {
+	case *ast.FunctionCall:
+		w.s.scanFunctionCall(n, w.result)
+		w.s.scanCondition(n.Filter, w.result)
+	case *ast.TableReference:
+		w.s.scanTableReference(n, w.result)
+	case *ast.SetOperation:
+		// Direct operands are already checked by the outer operation.
+		// Operations inside CTEs and subqueries start a new check.
+		if _, nested := parent.(*ast.SetOperation); !nested {
+			w.s.checkUnionInjection(n, false, w.result)
+		}
+	case *ast.SelectStatement:
+		w.s.scanCondition(n.Where, w.result)
+		w.s.scanCondition(n.Having, w.result)
+		w.s.scanCondition(n.Qualify, w.result)
+		w.s.scanCondition(n.PrewhereClause, w.result)
+		w.s.scanCondition(n.StartWith, w.result)
+	case *ast.ConnectByClause:
+		w.s.scanCondition(n.Condition, w.result)
+	case *ast.JoinClause:
+		w.s.scanCondition(n.Condition, w.result)
+		w.s.scanCondition(n.MatchCondition, w.result)
+	case *ast.UpdateStatement:
+		w.s.scanCondition(n.Where, w.result)
+	case *ast.Update:
+		w.s.scanCondition(n.Where, w.result)
+	case *ast.DeleteStatement:
+		w.s.scanCondition(n.Where, w.result)
+	case *ast.Delete:
+		w.s.scanCondition(n.Where, w.result)
+	case *ast.MergeStatement:
+		w.s.scanCondition(n.OnCondition, w.result)
+	case *ast.MergeWhenClause:
+		w.s.scanCondition(n.Condition, w.result)
+	case *ast.MergeAction:
+		w.s.scanCondition(n.Where, w.result)
+		w.s.scanCondition(n.DeleteWhere, w.result)
+	case *ast.WhenClause:
+		w.s.scanCondition(n.Condition, w.result)
+	case *ast.OnConflict:
+		w.s.scanCondition(n.TargetWhere, w.result)
+		w.s.scanCondition(n.Action.Where, w.result)
+	case *ast.Values:
+		w.s.scanStatementText(n.SQL(), w.result)
+		return false
+	case *ast.UtilityStatement:
+		w.s.scanStatementText(n.Text, w.result)
+		return false
+	case *ast.SetStatement:
+		w.s.scanStatementText(n.Text, w.result)
+		return false
+	case *ast.TransactionStatement:
+		w.s.scanStatementText(n.Text, w.result)
+		return false
+	case *ast.UseStatement:
+		w.s.scanStatementText(n.Text, w.result)
+		return false
+	case *ast.UnsupportedStatement:
+		w.s.scanStatementText(n.RawSQL, w.result)
+		return false
+	}
+	w.ancestors = append(w.ancestors, node)
+	return true
+}

pkg/sql/security/scan_walker_equiv_test.go

@@ -0,0 +1,236 @@
+// Copyright 2026 GoSQLX Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package security
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/ajitpratap0/GoSQLX/internal/benchcorpus"
+	"github.com/ajitpratap0/GoSQLX/pkg/gosqlx"
+	"github.com/ajitpratap0/GoSQLX/pkg/sql/ast"
+	"github.com/ajitpratap0/GoSQLX/pkg/sql/keywords"
+)
+
+// frozen pre-W6-02 implementation, the reference for TestScanWalkerEquivalence; do not edit
+func scanInspectReference(s *Scanner, tree *ast.AST) *ScanResult {
+	result := &ScanResult{
+		Findings: make([]Finding, 0),
+	}
+
+	if tree == nil {
+		return result
+	}
+
+	// Ancestry is local to the scan, so concurrent scans share no walk state.
+	// Preallocated so typical nesting depths need no regrowth.
+	ancestors := make([]ast.Node, 0, 16)
+	ast.Inspect(tree, func(node ast.Node) bool {
+		if node == nil {
+			ancestors = ancestors[:len(ancestors)-1]
+			return false
+		}
+		var parent ast.Node
+		if len(ancestors) > 0 {
+			parent = ancestors[len(ancestors)-1]
+		}
+		if join, ok := parent.(*ast.JoinClause); ok && node == &join.Left {
+			// Left repeats FROM or an earlier join's right reference,
+			// including its subquery and table function.
+			return false
+		}
+		switch n := node.(type) {
+		case *ast.FunctionCall:
+			s.scanFunctionCall(n, result)
+			s.scanCondition(n.Filter, result)
+		case *ast.TableReference:
+			s.scanTableReference(n, result)
+		case *ast.SetOperation:
+			// Direct operands are already checked by the outer operation.
+			// Operations inside CTEs and subqueries start a new check.
+			if _, nested := parent.(*ast.SetOperation); !nested {
+				s.checkUnionInjection(n, false, result)
+			}
+		case *ast.SelectStatement:
+			s.scanCondition(n.Where, result)
+			s.scanCondition(n.Having, result)
+			s.scanCondition(n.Qualify, result)
+			s.scanCondition(n.PrewhereClause, result)
+			s.scanCondition(n.StartWith, result)
+		case *ast.ConnectByClause:
+			s.scanCondition(n.Condition, result)
+		case *ast.JoinClause:
+			s.scanCondition(n.Condition, result)
+			s.scanCondition(n.MatchCondition, result)
+		case *ast.UpdateStatement:
+			s.scanCondition(n.Where, result)
+		case *ast.Update:
+			s.scanCondition(n.Where, result)
+		case *ast.DeleteStatement:
+			s.scanCondition(n.Where, result)
+		case *ast.Delete:
+			s.scanCondition(n.Where, result)
+		case *ast.MergeStatement:
+			s.scanCondition(n.OnCondition, result)
+		case *ast.MergeWhenClause:
+			s.scanCondition(n.Condition, result)
+		case *ast.MergeAction:
+			s.scanCondition(n.Where, result)
+			s.scanCondition(n.DeleteWhere, result)
+		case *ast.WhenClause:
+			s.scanCondition(n.Condition, result)
+		case *ast.OnConflict:
+			s.scanCondition(n.TargetWhere, result)
+			s.scanCondition(n.Action.Where, result)
+		case *ast.Values:
+			s.scanStatementText(n.SQL(), result)
+			return false
+		case *ast.UtilityStatement:
+			s.scanStatementText(n.Text, result)
+			return false
+		case *ast.SetStatement:
+			s.scanStatementText(n.Text, result)
+			return false
+		case *ast.TransactionStatement:
+			s.scanStatementText(n.Text, result)
+			return false
+		case *ast.UseStatement:
+			s.scanStatementText(n.Text, result)
+			return false
+		case *ast.UnsupportedStatement:
+			s.scanStatementText(n.RawSQL, result)
+			return false
+		}
+		ancestors = append(ancestors, node)
+		return true
+	})
+
+	// Update counts
+	s.updateCounts(result)
+
+	return result
+}
+
+type scanWalkerCase struct {
+	name    string
+	sql     string
+	dialect keywords.SQLDialect
+}
+
+// scanWalkerTargetCases is also used by the concurrent pool test. Keep the
+// small tree immediately after the deep tree to exercise the next scan.
+func scanWalkerTargetCases() []scanWalkerCase {
+	const pg = keywords.DialectPostgreSQL
+	const ch = keywords.DialectClickHouse
+	return []scanWalkerCase{
+		{"table function join", "SELECT * FROM url('http://x') AS a JOIN s3('y') AS b ON a.id = b.id", ch},
+		{"three table functions", "SELECT * FROM url('http://x') AS a JOIN s3('y') AS b ON a.id = b.id JOIN remote('z') AS c ON b.id = c.id", ch},
+		{"CTE set operations", "WITH c AS (SELECT a FROM t UNION SELECT b FROM u UNION SELECT NULL, NULL FROM information_schema.tables) SELECT * FROM c", pg},
+		{"subquery set operations", "SELECT * FROM (SELECT a FROM t UNION SELECT b FROM u UNION SELECT NULL, NULL FROM information_schema.tables) AS c", pg},
+		{"right set operation", "SELECT a FROM t UNION (SELECT b FROM u UNION SELECT NULL, NULL FROM information_schema.tables)", pg},
+		{"nested VALUES", "SELECT * FROM (VALUES (pg_sleep(5))) v", pg},
+		{"utility text", "EXEC xp_cmdshell 'dir'", keywords.DialectSQLServer},
+		{"set text", "SET @a = SLEEP(5)", keywords.DialectMySQL},
+		{"transaction text", "COMMIT PREPARED 'OR 1=1'", pg},
+		{"use text", "USE `OR 1=1`", keywords.DialectMySQL},
+		{"VALUES text", "VALUES (pg_sleep(5))", pg},
+		{"merge condition", "MERGE INTO t USING s ON t.id = s.other_id WHEN MATCHED AND 1=1 THEN UPDATE SET x = s.x", pg},
+		{"conflict condition", "INSERT INTO t VALUES (1) ON CONFLICT (id) DO UPDATE SET x = 1 WHERE 1=1", pg},
+		{"START WITH condition", "SELECT * FROM t START WITH 1=1 CONNECT BY PRIOR id = pid", keywords.DialectOracle},
+		{"deep chain", "SELECT * FROM t WHERE " + strings.Repeat("1=1 OR ", 4999) + "1=1", pg},
+		{"small after deep", "SELECT pg_sleep(5)", pg},
+	}
+}
+
+func scanWalkerResultDump(r *ScanResult) string {
+	var b strings.Builder
+	fmt.Fprintf(&b, "total=%d critical=%d high=%d medium=%d low=%d\n", r.TotalCount, r.CriticalCount, r.HighCount, r.MediumCount, r.LowCount)
+	for i, f := range r.Findings {
+		fmt.Fprintf(&b, "%d: Severity=%q Pattern=%q Line=%d Column=%d Description=%q Risk=%q SQL=%q Suggestion=%q\n", i, f.Severity, f.Pattern, f.Line, f.Column, f.Description, f.Risk, f.SQL, f.Suggestion)
+	}
+	return b.String()
+}
+
+func TestScanWalkerEquivalence(t *testing.T) {
+	s := NewScanner()
+	compare := func(t *testing.T, tree *ast.AST, requireFinding bool) (int, int) {
+		t.Helper()
+		got, want := s.Scan(tree), scanInspectReference(s, tree)
+		if !reflect.DeepEqual(got, want) {
+			t.Fatalf("Scan/reference mismatch\nScan:\n%sreference:\n%s", scanWalkerResultDump(got), scanWalkerResultDump(want))
+		}
+		if again := s.Scan(tree); !reflect.DeepEqual(got, again) {
+			t.Fatalf("Scan changed across calls\nfirst:\n%ssecond:\n%s", scanWalkerResultDump(got), scanWalkerResultDump(again))
+		}
+		if requireFinding && len(got.Findings) == 0 {
+			t.Fatal("targeted fixture must produce at least one finding")
+		}
+		return len(got.Findings), len(want.Findings)
+	}
+	parseCompare := func(t *testing.T, sql string, dialect keywords.SQLDialect, requireFinding bool) (int, int) {
+		t.Helper()
+		tree, err := gosqlx.ParseWithDialect(sql, dialect)
+		if err != nil {
+			if requireFinding {
+				t.Fatalf("targeted fixture parse: %v", err)
+			}
+			t.Skipf("unparseable corpus statement: %v", err)
+		}
+		defer ast.ReleaseAST(tree)
+		return compare(t, tree, requireFinding)
+	}
+	corpus, err := benchcorpus.Default()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stride := 1
+	if raceEnabled || testing.Short() {
+		stride = 4
+	}
+	t.Run("corpus", func(t *testing.T) {
+		for i, st := range corpus.All() {
+			if i%stride != 0 {
+				continue
+			}
+			t.Run(fmt.Sprintf("%s/%d", st.Dialect, i), func(t *testing.T) {
+				parseCompare(t, st.SQL, keywords.SQLDialect(st.Dialect), false)
+			})
+		}
+	})
+	total, referenceTotal := 0, 0
+	t.Run("realworld suffixes", func(t *testing.T) {
+		suffixes := []string{"", " OR 1=1", " OR a=a", " UNION SELECT NULL,NULL FROM information_schema.tables", "; DROP TABLE x", " AND SLEEP(5)", " OR 'a'='a'"}
+		for _, dialect := range benchcorpus.Dialects() {
+			for i, st := range corpus.RealWorld(dialect) {
+				if i%stride != 0 {
+					continue
+				}
+				for j, suffix := range suffixes {
+					t.Run(fmt.Sprintf("%s/%d/%d", dialect, i, j), func(t *testing.T) {
+						n, ref := parseCompare(t, st.SQL+suffix, keywords.SQLDialect(dialect), false)
+						total += n
+						referenceTotal += ref
+					})
+				}
+			}
+		}
+	})
+	if total == 0 || referenceTotal == 0 {
+		t.Fatalf("realworld suffix matrix is vacuous: Scan findings=%d reference findings=%d", total, referenceTotal)
+	}
+	t.Run("targeted", func(t *testing.T) {
+		for _, tc := range scanWalkerTargetCases() {
+			t.Run(tc.name, func(t *testing.T) { parseCompare(t, tc.sql, tc.dialect, true) })
+		}
+		t.Run("unsupported text", func(t *testing.T) {
+			// The parser never constructs UnsupportedStatement; cover this legacy
+			// node directly, as TestScan_RawStatementNodeTypes does.
+			tree := &ast.AST{Statements: []ast.Statement{&ast.UnsupportedStatement{RawSQL: "SELECT pg_sleep(5)"}}}
+			defer ast.ReleaseAST(tree)
+			compare(t, tree, true)
+		})
+	})
+}