From 3b145339d0d71f001c279504d695268b390e09ea Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:47:50 +0530 Subject: [PATCH 1/3] perf(parser): currentToken is a pointer into p.tokens with a per-parser EOF sentinel [W3-02] advance copied a 72-byte pointerful TokenWithSpan into the Parser on every token, paying a GC write barrier per pointer word and a memmove. currentToken now points at p.tokens[currentPos], or at the parser's own zero EOF sentinel past the end of the stream (decision D4 option b), and peekToken returns a pointer. Every parse entry point, advance, seek, Reset and Release set the pointer, so a zero-value &Parser{} stays usable. Parse results, errors and positions are unchanged: the behaviour goldens pass and a recovery-path differential over the corpus, every dialect and truncated variants was byte-identical. New tests pin the cursor invariant, that the sentinel is per parser and never written, and the zero-value parser. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 1 + pkg/sql/parser/currenttoken_pointer_test.go | 321 ++++++++++++++++++++ pkg/sql/parser/ddl_columns.go | 2 +- pkg/sql/parser/dialect_migration_test.go | 5 +- pkg/sql/parser/expressions_ops.go | 7 +- pkg/sql/parser/paren_query_test.go | 2 +- pkg/sql/parser/parser.go | 48 ++- pkg/sql/parser/recovery.go | 4 +- pkg/sql/parser/type_helpers_test.go | 4 +- pkg/sql/parser/utility_cursor_test.go | 4 +- pkg/sql/parser/utility_stmt.go | 4 +- 11 files changed, 374 insertions(+), 28 deletions(-) create mode 100644 pkg/sql/parser/currenttoken_pointer_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d251729d0..3a8fd3094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Performance - **Pools**: the tokenizer, parser and AST pools now count Get/Put through precreated, cache-line padded atomic counters (`internal/poolstats`) instead of an RWMutex-guarded map keyed by pool name, removing a lock, map lookup and string hash from every pool operation. `metrics.GetPoolStats` and `metrics.ResetPoolStats` behave exactly as before; named custom pools keep the map path. [W1-01] - **Parser**: token preprocessing now takes a type-gated fast path. Only identifier, keyword, aggregate and compound tokens take the normalisation path; every other token is appended with a single copy. Output is identical (guarded by an old-versus-new differential test); no API or behaviour change. +- **Parser**: the parser's current token is now a pointer into its token slice (with a per-parser zero EOF sentinel) instead of a 72-byte copy made on every `advance`, and `peekToken` returns a pointer, which removes most GC write-barrier work and struct copying from the parse loop. Parse results, errors and positions are byte-identical (behaviour goldens plus a recovery-path differential); no API change. A zero-value `&Parser{}` keeps working. [W3-02] ### Fixed - **AST**: `AlterTableStatement.SQL()` now separates multiple ALTER TABLE actions with commas, matching the compact formatter and preserving action boundaries when re-parsed. diff --git a/pkg/sql/parser/currenttoken_pointer_test.go b/pkg/sql/parser/currenttoken_pointer_test.go new file mode 100644 index 000000000..d8cda7689 --- /dev/null +++ b/pkg/sql/parser/currenttoken_pointer_test.go @@ -0,0 +1,321 @@ +// 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 ( + "sync" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/models" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" +) + +// currentTokenCorpus covers the parser paths that save, restore, seek over +// and peek past currentToken: speculative lookahead (ALTER TABLE, CREATE +// FUNCTION bodies, index hints), recovery after errors, ClickHouse mutations +// (which temporarily shorten p.tokens) and plain DML/DDL that reaches EOF. +var currentTokenCorpus = []string{ + "SELECT 1", + "SELECT a, b FROM t WHERE a = 1 AND b <> 2 ORDER BY a LIMIT 5", + "SELECT a <=> b FROM t", + "SELECT * FROM t1 JOIN t2 ON t1.id = t2.id LEFT JOIN t3 USING (id)", + "WITH c AS (SELECT 1) SELECT * FROM c UNION ALL SELECT 2", + "INSERT INTO t (a, b) VALUES (1, 2) RETURNING a", + "UPDATE t SET a = 1 FROM s WHERE t.id = s.id RETURNING a", + "DELETE FROM t WHERE id IN (SELECT id FROM s)", + "CREATE TABLE t (id INT PRIMARY KEY, name TEXT NOT NULL DEFAULT 'x')", + "ALTER TABLE t ADD COLUMN c INT, DROP COLUMN d", + "ALTER TABLE t UPDATE a = 1, b = 2 IN PARTITION p WHERE id > 0", + "SELECT a FROM t USE INDEX (i1) WHERE a = 1", + "SELECT INTERVAL '1' DAY TO SECOND", + "SELECT `a` FROM `t`; SELECT \"b\" FROM [c]", + "SELECT 1; SELECT 2; ;", + "SELECT FROM", + "SELECT a b c FROM", + "SELECT (", + "", + ";", +} + +func cursorTestTokens(t testing.TB, sql string) []models.TokenWithSpan { + t.Helper() + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + toks, err := tkz.Tokenize([]byte(sql)) + if err != nil { + // Malformed input is part of the corpus: an empty stream still exercises + // the cursor setup. + return nil + } + // The tokenizer owns its buffer; copy it so the slice outlives the pool. + return append([]models.TokenWithSpan(nil), toks...) +} + +// assertCursorInvariant checks the invariant every parser method relies on: +// currentToken is the token at currentPos, or the parser's own zero EOF +// sentinel once the cursor is past the stream. +func assertCursorInvariant(t *testing.T, p *Parser, ctx string) { + t.Helper() + if p.currentToken == nil { + t.Fatalf("%s: currentToken is nil", ctx) + } + if p.currentPos >= 0 && p.currentPos < len(p.tokens) { + if p.currentToken != &p.tokens[p.currentPos] { + t.Fatalf("%s: currentToken does not point at tokens[%d]", ctx, p.currentPos) + } + return + } + if p.currentToken != &p.eof { + t.Fatalf("%s: past the stream (pos %d of %d) currentToken is not the parser's EOF sentinel", ctx, p.currentPos, len(p.tokens)) + } + if p.eof != (models.TokenWithSpan{}) { + t.Fatalf("%s: EOF sentinel was modified: %+v", ctx, p.eof) + } +} + +// TestCurrentToken_TracksCursor walks every position of several streams with +// advance and seek, and checks currentToken follows currentPos exactly, +// including saved/restored cursors and the move past EOF. +func TestCurrentToken_TracksCursor(t *testing.T) { + for _, sql := range currentTokenCorpus { + sql := sql + t.Run(sql, func(t *testing.T) { + p := NewParser() + defer p.Release() + toks := p.preprocess(cursorTestTokens(t, sql)) + p.tokens = toks + p.currentPos = 0 + p.seek(0) + assertCursorInvariant(t, p, "seek(0)") + + for i := 0; i <= len(toks)+1; i++ { + assertCursorInvariant(t, p, "advance") + // Save/restore, as the speculative-lookahead helpers do. + savedPos, savedTok := p.currentPos, p.currentToken + p.advance() + p.advance() + p.currentPos, p.currentToken = savedPos, savedTok + assertCursorInvariant(t, p, "restore") + p.advance() + } + for i := len(toks) + 1; i >= -1; i-- { + p.seek(i) + if i < 0 { + if p.currentPos != len(toks) { + t.Fatalf("seek(%d): pos = %d, want EOF %d", i, p.currentPos, len(toks)) + } + } + assertCursorInvariant(t, p, "seek") + } + }) + } +} + +// TestCurrentToken_PeekToken checks peekToken against the token slice and +// that past the end it returns the zero sentinel without modifying it. +func TestCurrentToken_PeekToken(t *testing.T) { + p := NewParser() + defer p.Release() + toks := p.preprocess(cursorTestTokens(t, "SELECT a FROM t")) + p.tokens = toks + p.seek(0) + for i := range toks { + p.seek(i) + got := p.peekToken() + if got == nil { + t.Fatalf("peekToken at %d returned nil", i) + } + if i+1 < len(toks) { + if *got != toks[i+1] { + t.Errorf("peekToken at %d = %+v, want %+v", i, *got, toks[i+1]) + } + } else if *got != (models.TokenWithSpan{}) { + t.Errorf("peekToken past end = %+v, want zero token", *got) + } + } + if p.eof != (models.TokenWithSpan{}) { + t.Errorf("EOF sentinel modified by peekToken: %+v", p.eof) + } +} + +// TestCurrentToken_SentinelUnmodifiedAfterParsing runs every parse entry +// point over the corpus, including error and recovery paths and parsers that +// were pooled and reset in between, and requires the per-parser EOF sentinel +// to stay the zero token. A write through currentToken at EOF would corrupt +// every later end-of-input check on that parser. +func TestCurrentToken_SentinelUnmodifiedAfterParsing(t *testing.T) { + p := GetParser() + defer PutParser(p) + for round := 0; round < 3; round++ { + for _, sql := range currentTokenCorpus { + toks := cursorTestTokens(t, sql) + + if tree, err := p.ParseFromModelTokens(toks); err == nil { + ast.ReleaseAST(tree) + } + assertSentinelClean(t, p, "ParseFromModelTokens", sql) + + p.ParseWithRecoveryFromModelTokens(toks) + assertSentinelClean(t, p, "ParseWithRecoveryFromModelTokens", sql) + + if tree, err := p.ParseContextFromModelTokens(t.Context(), toks); err == nil { + ast.ReleaseAST(tree) + } + assertSentinelClean(t, p, "ParseContextFromModelTokens", sql) + + p.Reset() + assertSentinelClean(t, p, "Reset", sql) + } + } +} + +func assertSentinelClean(t *testing.T, p *Parser, op, sql string) { + t.Helper() + if p.eof != (models.TokenWithSpan{}) { + t.Fatalf("%s(%q): EOF sentinel modified: %+v", op, sql, p.eof) + } + if p.currentToken == nil { + t.Fatalf("%s(%q): currentToken is nil", op, sql) + } +} + +// TestCurrentToken_SentinelIsPerParser guards decision D4: the sentinel must +// never be shared between parsers, or a stray write would race across +// goroutines and leak between tests. +func TestCurrentToken_SentinelIsPerParser(t *testing.T) { + a, b := NewParser(), NewParser() + if a.currentToken == b.currentToken { + t.Fatal("two parsers share one EOF sentinel") + } + if a.currentToken != &a.eof || b.currentToken != &b.eof { + t.Fatal("a fresh parser's currentToken is not its own sentinel") + } + pa, pb := GetParser(), GetParser() + defer PutParser(pa) + defer PutParser(pb) + if pa.currentToken == pb.currentToken { + t.Fatal("two pooled parsers share one EOF sentinel") + } +} + +// TestCurrentToken_ZeroValueParser covers &Parser{}, which the type's +// documentation allows: every entry point must set up the cursor itself, +// including for an empty token stream, and advance/seek must be safe on a +// parser that was never initialised. +func TestCurrentToken_ZeroValueParser(t *testing.T) { + for _, sql := range currentTokenCorpus { + sql := sql + t.Run(sql, func(t *testing.T) { + toks := cursorTestTokens(t, sql) + if tree, err := (&Parser{}).ParseFromModelTokens(toks); err == nil { + ast.ReleaseAST(tree) + } + (&Parser{}).ParseWithRecoveryFromModelTokens(toks) + if tree, err := (&Parser{}).ParseContextFromModelTokens(t.Context(), toks); err == nil { + ast.ReleaseAST(tree) + } + }) + } + + p := &Parser{tokens: []models.TokenWithSpan{mkSpanToken(models.TokenTypeSelect, "SELECT")}} + p.advance() + assertCursorInvariant(t, p, "zero-value advance") + p = &Parser{tokens: []models.TokenWithSpan{mkSpanToken(models.TokenTypeSelect, "SELECT")}} + p.seek(0) + assertCursorInvariant(t, p, "zero-value seek") + p = &Parser{} + if tree, err := p.ParseFromModelTokens(nil); err == nil { + ast.ReleaseAST(tree) + } + assertCursorInvariant(t, p, "zero-value empty stream") +} + +// TestCurrentToken_EmptyStreamClearsStaleCursor: a reused parser given an +// empty stream must not keep pointing into the previous query's tokens. +func TestCurrentToken_EmptyStreamClearsStaleCursor(t *testing.T) { + p := NewParser() + defer p.Release() + first := cursorTestTokens(t, "SELECT a FROM t") + if tree, err := p.ParseFromModelTokens(first); err == nil { + ast.ReleaseAST(tree) + } + if tree, err := p.ParseFromModelTokens(nil); err == nil { + ast.ReleaseAST(tree) + } + assertCursorInvariant(t, p, "empty stream after a parse") + p.ParseWithRecoveryFromModelTokens(first) + p.ParseWithRecoveryFromModelTokens(nil) + assertCursorInvariant(t, p, "empty recovery stream after a parse") +} + +// TestCurrentToken_ConcurrentParsers parses concurrently on independent +// parsers. Under -race this proves no EOF sentinel (or other cursor state) is +// shared between goroutines. +func TestCurrentToken_ConcurrentParsers(t *testing.T) { + var wg sync.WaitGroup + for g := 0; g < 8; g++ { + wg.Add(1) + go func() { + defer wg.Done() + p := GetParser() + defer PutParser(p) + for i := 0; i < 20; i++ { + for _, sql := range currentTokenCorpus { + tkz := tokenizer.GetTokenizer() + toks, err := tkz.Tokenize([]byte(sql)) + if err == nil { + if tree, perr := p.ParseFromModelTokens(toks); perr == nil { + ast.ReleaseAST(tree) + } + p.ParseWithRecoveryFromModelTokens(toks) + } + tokenizer.PutTokenizer(tkz) + } + } + }() + } + wg.Wait() +} + +// TestCurrentToken_NextAdjacent pins nextAdjacent, whose rewrite from token +// copies to End/Start comparisons must keep the same answers. +func TestCurrentToken_NextAdjacent(t *testing.T) { + cases := []struct { + sql string + pos int // seek here, then ask about offsets 1 and 2 + want1 bool + want2 bool + reason string + }{ + {"a<=>b", 0, true, true, "operator written as one token"}, + {"a < = > b", 0, false, false, "spaces between pieces"}, + {"a<= >b", 0, true, false, "gap before the third piece"}, + {"a<= >b", 2, true, true, "cursor on the middle piece, no gaps after it"}, + {"a<=> b", 0, true, false, "gap before the last token"}, + } + for _, tc := range cases { + p := NewParser() + p.tokens = p.preprocess(cursorTestTokens(t, tc.sql)) + p.seek(tc.pos) + if got := p.nextAdjacent(1); got != tc.want1 { + t.Errorf("%q @%d nextAdjacent(1) = %v, want %v (%s)", tc.sql, tc.pos, got, tc.want1, tc.reason) + } + if got := p.nextAdjacent(2); got != tc.want2 { + t.Errorf("%q @%d nextAdjacent(2) = %v, want %v (%s)", tc.sql, tc.pos, got, tc.want2, tc.reason) + } + p.Release() + } +} diff --git a/pkg/sql/parser/ddl_columns.go b/pkg/sql/parser/ddl_columns.go index 9a0622e29..c81a2fa13 100644 --- a/pkg/sql/parser/ddl_columns.go +++ b/pkg/sql/parser/ddl_columns.go @@ -490,7 +490,7 @@ func (p *Parser) writeRawLeaf(buf *strings.Builder, tok models.Token, afterWord writeDelimitedToken(buf, val, '\'') return nil case isQuotedTokenType(tok): - p.writeTokenSource(buf, &p.currentToken) + p.writeTokenSource(buf, p.currentToken) default: buf.WriteString(val) } diff --git a/pkg/sql/parser/dialect_migration_test.go b/pkg/sql/parser/dialect_migration_test.go index d1a6b98a4..978fa1f1e 100644 --- a/pkg/sql/parser/dialect_migration_test.go +++ b/pkg/sql/parser/dialect_migration_test.go @@ -18,6 +18,7 @@ import ( "strings" "testing" + "github.com/ajitpratap0/GoSQLX/pkg/models" "github.com/ajitpratap0/GoSQLX/pkg/sql/dialect" "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" ) @@ -174,7 +175,7 @@ func TestMigration_IsQualifyKeyword_Snowflake(t *testing.T) { t.Parallel() // Wire up a minimal parser with the current token set to "qualify". p := NewParser(WithDialect(tc.dialect)) - p.currentToken.Token.Value = "QUALIFY" + p.currentToken = &models.TokenWithSpan{Token: models.Token{Value: "QUALIFY"}} if got := p.isQualifyKeyword(); got != tc.want { t.Errorf("isQualifyKeyword() with dialect=%q = %v, want %v", tc.dialect, got, tc.want) @@ -208,7 +209,7 @@ func TestMigration_IsMatchRecognizeKeyword(t *testing.T) { t.Run(tc.dialect, func(t *testing.T) { t.Parallel() p := NewParser(WithDialect(tc.dialect)) - p.currentToken.Token.Value = "MATCH_RECOGNIZE" + p.currentToken = &models.TokenWithSpan{Token: models.Token{Value: "MATCH_RECOGNIZE"}} if got := p.isMatchRecognizeKeyword(); got != tc.want { t.Errorf("isMatchRecognizeKeyword() with dialect=%q = %v, want %v", tc.dialect, got, tc.want) diff --git a/pkg/sql/parser/expressions_ops.go b/pkg/sql/parser/expressions_ops.go index c493f1a6b..33ba684f6 100644 --- a/pkg/sql/parser/expressions_ops.go +++ b/pkg/sql/parser/expressions_ops.go @@ -49,12 +49,11 @@ func (p *Parser) peekTypeAt(offset int) models.TokenType { // where the current one ends, so that two tokens the tokenizer split apart // (for example "<=" and ">") really were written as one operator ("<=>"). func (p *Parser) nextAdjacent(offset int) bool { - prev := p.currentToken + prevEnd := p.currentToken.End if offset > 1 { - prev = p.peekAt(offset - 1) + prevEnd = p.peekAt(offset - 1).End } - next := p.peekAt(offset) - return prev.End == next.Start + return prevEnd == p.peekAt(offset).Start } // peekValueIs reports whether the token offset positions ahead has the given diff --git a/pkg/sql/parser/paren_query_test.go b/pkg/sql/parser/paren_query_test.go index 178209ec3..743d46856 100644 --- a/pkg/sql/parser/paren_query_test.go +++ b/pkg/sql/parser/paren_query_test.go @@ -632,7 +632,7 @@ func TestParenQuery_ClassifierNoAlloc(t *testing.T) { toks := p.preprocess(raw) p.tokens = toks p.currentPos = 0 - p.currentToken = toks[0] + p.currentToken = &toks[0] if allocs := testing.AllocsPerRun(10, func() { _ = p.parenOpensQueryExpression() }); allocs != 0 { t.Errorf("%q: parenOpensQueryExpression allocated %.0f times", sql, allocs) } diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 6025d51d5..8dd72edc0 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -67,6 +67,7 @@ type TokenPosition struct { var parserPool = sync.Pool{ New: func() interface{} { p := &Parser{} + p.currentToken = &p.eof p.resetDialectState() return p }, @@ -159,7 +160,7 @@ func (p *Parser) Reset() { p.currentPos = 0 p.implicitEnd = 0 p.openBlockWork = 0 - p.currentToken = models.TokenWithSpan{} + p.currentToken = &p.eof p.depth = 0 p.ctx = nil p.strict = false @@ -219,7 +220,8 @@ const MaxRecursionDepth = 100 // - tokenBuf: Parser-owned scratch buffer holding the preprocessed stream, // reused across parses and scrubbed (or dropped when large) by Reset // - currentPos: Current position in token stream -// - currentToken: Current token being examined +// - currentToken: Pointer to the current token in tokens (or to the per-parser +// zero EOF sentinel past the end); read-only // - depth: Recursion depth counter (DoS protection via MaxRecursionDepth) // - ctx: Optional context for cancellation support // - positions: Source position mapping for enhanced error reporting @@ -316,9 +318,14 @@ 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 - currentPos int - currentToken models.TokenWithSpan + 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 + // bytes instead of a 72 B pointerful struct (which cost a GC write barrier + // per word). The pointee is read-only: tokens are never mutated while + // parsing, and nothing may write through currentToken (see eof). + currentToken *models.TokenWithSpan depth int // Current recursion depth ctx context.Context // Optional context for cancellation support strict bool // Strict mode rejects empty statements @@ -373,6 +380,13 @@ type Parser struct { // examined before running into the end of the input inside an unclosed // block. It bounds that wasted work (see utilityStatementEnd). openBlockWork int + // eof is this parser's end-of-input sentinel: always the zero token. It is + // per parser, not a package variable, so a stray write can neither race + // across goroutines nor leak into other parsers. Every parse entry point, + // advance, seek, Reset and Release point currentToken at it when no real + // token applies, which is also what makes a zero-value &Parser{} usable. + // Nothing may write through currentToken. + eof models.TokenWithSpan } // Deprecated: Parse is provided for backward compatibility only and is scheduled for @@ -421,7 +435,9 @@ func (p *Parser) parseTokens(tokens []models.TokenWithSpan) (*ast.AST, error) { p.implicitEnd = 0 p.openBlockWork = 0 if len(tokens) > 0 { - p.currentToken = tokens[0] + p.currentToken = &tokens[0] + } else { + p.currentToken = &p.eof } // Get a pre-allocated AST from the pool @@ -651,7 +667,9 @@ func (p *Parser) parseContextTokens(ctx context.Context, tokens []models.TokenWi p.implicitEnd = 0 p.openBlockWork = 0 if len(tokens) > 0 { - p.currentToken = tokens[0] + p.currentToken = &tokens[0] + } else { + p.currentToken = &p.eof } // Get a pre-allocated AST from the pool @@ -713,7 +731,7 @@ func (p *Parser) Release() { p.tokens = nil p.tokenBuf = nil p.currentPos = 0 - p.currentToken = models.TokenWithSpan{} + p.currentToken = &p.eof p.depth = 0 p.ctx = nil } @@ -993,6 +1011,7 @@ func (p *Parser) parseSnowflakeStageStatement(kind string) (ast.Statement, error // NewParser creates a new parser with optional configuration. func NewParser(opts ...ParserOption) *Parser { p := &Parser{} + p.currentToken = &p.eof p.resetDialectState() for _, opt := range opts { opt(p) @@ -1036,20 +1055,23 @@ func (p *Parser) checkStrictEmptySemicolon() error { func (p *Parser) advance() { p.currentPos++ if p.currentPos < len(p.tokens) { - p.currentToken = p.tokens[p.currentPos] + p.currentToken = &p.tokens[p.currentPos] } else { - p.currentToken = models.TokenWithSpan{} // EOF sentinel + p.currentToken = &p.eof // EOF sentinel } } // peekToken returns the next token without advancing the parser position. // Returns an empty TokenWithSpan if at the end of input. -func (p *Parser) peekToken() models.TokenWithSpan { +// +// The result points into p.tokens (or at the parser's zero EOF sentinel) and +// is read-only. +func (p *Parser) peekToken() *models.TokenWithSpan { nextPos := p.currentPos + 1 if nextPos < len(p.tokens) { - return p.tokens[nextPos] + return &p.tokens[nextPos] } - return models.TokenWithSpan{} + return &p.eof } // ============================================================================= diff --git a/pkg/sql/parser/recovery.go b/pkg/sql/parser/recovery.go index 6185f84f3..f52b01c89 100644 --- a/pkg/sql/parser/recovery.go +++ b/pkg/sql/parser/recovery.go @@ -188,7 +188,9 @@ func (p *Parser) parseWithRecovery(tokens []models.TokenWithSpan) ([]ast.Stateme p.implicitEnd = 0 p.openBlockWork = 0 if len(tokens) > 0 { - p.currentToken = tokens[0] + p.currentToken = &tokens[0] + } else { + p.currentToken = &p.eof } statements := make([]ast.Statement, 0, 8) diff --git a/pkg/sql/parser/type_helpers_test.go b/pkg/sql/parser/type_helpers_test.go index 237e1fcb4..7899e1c72 100644 --- a/pkg/sql/parser/type_helpers_test.go +++ b/pkg/sql/parser/type_helpers_test.go @@ -67,7 +67,7 @@ func TestIsAnyType(t *testing.T) { p := &Parser{ tokens: tokens, currentPos: 0, - currentToken: tokens[0], + currentToken: &tokens[0], } result := p.isAnyType(tt.types...) if result != tt.expected { @@ -113,7 +113,7 @@ func TestMatchType(t *testing.T) { p := &Parser{ tokens: tt.tokens, currentPos: 0, - currentToken: tt.tokens[0], + currentToken: &tt.tokens[0], } result := p.matchType(tt.matchAgainst) if result != tt.wantMatch { diff --git a/pkg/sql/parser/utility_cursor_test.go b/pkg/sql/parser/utility_cursor_test.go index 3b4ff8827..2ed210683 100644 --- a/pkg/sql/parser/utility_cursor_test.go +++ b/pkg/sql/parser/utility_cursor_test.go @@ -115,7 +115,7 @@ func TestParserSeekNegativeIndex(t *testing.T) { if p.currentPos != len(p.tokens) { t.Errorf("position = %d, want EOF at %d", p.currentPos, len(p.tokens)) } - if p.currentToken != (models.TokenWithSpan{}) { - t.Errorf("token = %+v, want zero EOF token", p.currentToken) + if *p.currentToken != (models.TokenWithSpan{}) { + t.Errorf("token = %+v, want zero EOF token", *p.currentToken) } } diff --git a/pkg/sql/parser/utility_stmt.go b/pkg/sql/parser/utility_stmt.go index d2bab6d41..d8ab040db 100644 --- a/pkg/sql/parser/utility_stmt.go +++ b/pkg/sql/parser/utility_stmt.go @@ -480,9 +480,9 @@ func (p *Parser) seek(i int) { } p.currentPos = i if i < len(p.tokens) { - p.currentToken = p.tokens[i] + p.currentToken = &p.tokens[i] } else { - p.currentToken = models.TokenWithSpan{} + p.currentToken = &p.eof } } From 5e64df2be32b1001941385befee937551074b67a Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:28:22 +0530 Subject: [PATCH 2/3] test(parser): pin currentToken re-point on Reset/Release and sentinel over the full corpus [W3-02] Require currentToken == &p.eof after Reset and Release, cover an oversized tokenBuf, the pool New init, peekToken sentinel identity and the empty ParseContextFromModelTokens stream, and drive the sentinel-unmodified check over the W0-02 corpus for every dialect through all three entry points. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- pkg/sql/parser/currenttoken_corpus_test.go | 61 +++++++++++++++ pkg/sql/parser/currenttoken_pointer_test.go | 84 ++++++++++++++++++++- pkg/sql/parser/export_test.go | 7 ++ 3 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 pkg/sql/parser/currenttoken_corpus_test.go diff --git a/pkg/sql/parser/currenttoken_corpus_test.go b/pkg/sql/parser/currenttoken_corpus_test.go new file mode 100644 index 000000000..76f98fdf7 --- /dev/null +++ b/pkg/sql/parser/currenttoken_corpus_test.go @@ -0,0 +1,61 @@ +// Copyright 2026 GoSQLX Authors +// SPDX-License-Identifier: Apache-2.0 + +package parser_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/internal/goldens" + "github.com/ajitpratap0/GoSQLX/pkg/models" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" + "github.com/ajitpratap0/GoSQLX/pkg/sql/parser" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" +) + +// TestCurrentTokenSentinelUnmodifiedOverCorpus drives all three token entry +// points over the whole W0-02 benchmark corpus under every dialect and +// requires the per-parser EOF sentinel to stay the zero token (PLAN W3-02). +// This reaches dialect-specific parse paths where a future write through +// p.currentToken would corrupt end-of-input checks. After each statement the +// parser is reset and must hold no pointer into the token slice. +func TestCurrentTokenSentinelUnmodifiedOverCorpus(t *testing.T) { + stmts := goldens.Corpus(t) + for _, dn := range goldens.Dialects() { + d := keywords.SQLDialect(dn) + tkz, err := tokenizer.NewWithDialect(d) + if err != nil { + t.Fatalf("%s: tokenizer: %v", dn, err) + } + p := parser.NewParser(parser.WithDialect(dn)) + for _, st := range stmts { + toks, terr := tkz.Tokenize([]byte(st.SQL)) + if terr != nil { + tkz.Reset() + continue + } + toks = append([]models.TokenWithSpan(nil), toks...) + tkz.Reset() + + if tree, err := p.ParseFromModelTokens(toks); err == nil { + ast.ReleaseAST(tree) + } + p.ParseWithRecoveryFromModelTokens(toks) + if tree, err := p.ParseContextFromModelTokens(t.Context(), toks); err == nil { + ast.ReleaseAST(tree) + } + if !parser.EOFSentinelClean(p) { + t.Fatalf("dialect %s, %s#%d: EOF sentinel modified", dn, st.File, st.Index) + } + p.Reset() + if !parser.EOFSentinelClean(p) || !parser.CursorRepointed(p) { + t.Fatalf("dialect %s, %s#%d: Reset left a stale cursor or dirty sentinel", dn, st.File, st.Index) + } + } + p.Release() + if !parser.CursorRepointed(p) { + t.Fatalf("dialect %s: Release left a stale cursor", dn) + } + } +} diff --git a/pkg/sql/parser/currenttoken_pointer_test.go b/pkg/sql/parser/currenttoken_pointer_test.go index d8cda7689..0b0f1cf82 100644 --- a/pkg/sql/parser/currenttoken_pointer_test.go +++ b/pkg/sql/parser/currenttoken_pointer_test.go @@ -15,6 +15,7 @@ package parser import ( + "strings" "sync" "testing" @@ -142,8 +143,13 @@ func TestCurrentToken_PeekToken(t *testing.T) { if *got != toks[i+1] { t.Errorf("peekToken at %d = %+v, want %+v", i, *got, toks[i+1]) } - } else if *got != (models.TokenWithSpan{}) { - t.Errorf("peekToken past end = %+v, want zero token", *got) + } else { + if *got != (models.TokenWithSpan{}) { + t.Errorf("peekToken past end = %+v, want zero token", *got) + } + if got != &p.eof { + t.Errorf("peekToken past end at %d is not the parser's own EOF sentinel", i) + } } } if p.eof != (models.TokenWithSpan{}) { @@ -178,6 +184,7 @@ func TestCurrentToken_SentinelUnmodifiedAfterParsing(t *testing.T) { p.Reset() assertSentinelClean(t, p, "Reset", sql) + assertRepointedAtSentinel(t, p, "Reset", sql) } } } @@ -192,6 +199,66 @@ func assertSentinelClean(t *testing.T, p *Parser, op, sql string) { } } +// assertRepointedAtSentinel requires a reset/released parser to hold no +// interior pointer into a token slice: a stale currentToken would keep a +// dropped oversized tokenBuf or a caller-owned slice alive in the pool. +func assertRepointedAtSentinel(t *testing.T, p *Parser, op, sql string) { + t.Helper() + if p.currentToken != &p.eof { + t.Fatalf("%s(%q): currentToken is not re-pointed at the EOF sentinel (stale interior pointer)", op, sql) + } + if p.tokens != nil { + t.Fatalf("%s(%q): tokens still referenced", op, sql) + } +} + +// TestCurrentToken_ReleaseRepointsSentinel pins Release's re-point. +func TestCurrentToken_ReleaseRepointsSentinel(t *testing.T) { + for _, sql := range currentTokenCorpus { + p := NewParser() + toks := cursorTestTokens(t, sql) + if tree, err := p.ParseFromModelTokens(toks); err == nil { + ast.ReleaseAST(tree) + } + p.Release() + assertSentinelClean(t, p, "Release", sql) + assertRepointedAtSentinel(t, p, "Release", sql) + } +} + +// TestCurrentToken_ResetDropsOversizedBuffer parses a stream whose token +// buffer exceeds maxPooledTokenBuf and requires Reset to leave no pointer +// into the dropped buffer or the caller's slice. +func TestCurrentToken_ResetDropsOversizedBuffer(t *testing.T) { + var sb strings.Builder + sb.WriteString("SELECT 1") + for i := 0; i < maxPooledTokenBuf; i++ { + sb.WriteString(", 1") + } + toks := cursorTestTokens(t, sb.String()) + if len(toks) <= maxPooledTokenBuf { + t.Fatalf("test stream too small: %d tokens", len(toks)) + } + p := NewParser() + if tree, err := p.ParseFromModelTokens(toks); err != nil { + t.Fatal(err) + } else { + ast.ReleaseAST(tree) + } + big := cap(p.tokenBuf) > maxPooledTokenBuf + p.Reset() + assertRepointedAtSentinel(t, p, "Reset", "oversized") + if big && p.tokenBuf != nil { + t.Fatal("oversized tokenBuf was not dropped by Reset") + } + // Caller-owned slice (ParseWithPositions style): parse the raw slice + // directly, then reset. + p.tokens = toks + p.seek(len(toks) / 2) + p.Reset() + assertRepointedAtSentinel(t, p, "Reset", "caller-owned") +} + // TestCurrentToken_SentinelIsPerParser guards decision D4: the sentinel must // never be shared between parsers, or a stray write would race across // goroutines and leak between tests. @@ -209,6 +276,9 @@ func TestCurrentToken_SentinelIsPerParser(t *testing.T) { if pa.currentToken == pb.currentToken { t.Fatal("two pooled parsers share one EOF sentinel") } + if pa.currentToken != &pa.eof || pb.currentToken != &pb.eof { + t.Fatal("a pooled parser's currentToken is not its own sentinel") + } } // TestCurrentToken_ZeroValueParser covers &Parser{}, which the type's @@ -259,6 +329,16 @@ func TestCurrentToken_EmptyStreamClearsStaleCursor(t *testing.T) { p.ParseWithRecoveryFromModelTokens(first) p.ParseWithRecoveryFromModelTokens(nil) assertCursorInvariant(t, p, "empty recovery stream after a parse") + if tree, err := p.ParseContextFromModelTokens(t.Context(), first); err == nil { + ast.ReleaseAST(tree) + } + if tree, err := p.ParseContextFromModelTokens(t.Context(), nil); err == nil { + ast.ReleaseAST(tree) + } + assertCursorInvariant(t, p, "empty context stream after a parse") + if p.currentToken != &p.eof { + t.Fatal("empty context stream left a stale cursor") + } } // TestCurrentToken_ConcurrentParsers parses concurrently on independent diff --git a/pkg/sql/parser/export_test.go b/pkg/sql/parser/export_test.go index 53200011f..683d234c9 100644 --- a/pkg/sql/parser/export_test.go +++ b/pkg/sql/parser/export_test.go @@ -26,3 +26,10 @@ func BenchPreprocessOwned(toks []models.TokenWithSpan) []models.TokenWithSpan { func BenchParseTokens(p *Parser, toks []models.TokenWithSpan) (*ast.AST, error) { return p.parseTokens(toks) } + +// EOFSentinelClean reports whether p's EOF sentinel is still the zero token. +func EOFSentinelClean(p *Parser) bool { return p.eof == (models.TokenWithSpan{}) } + +// CursorRepointed reports whether p holds no pointer into a token slice, i.e. +// currentToken is its own EOF sentinel (the state after Reset or Release). +func CursorRepointed(p *Parser) bool { return p.currentToken == &p.eof && p.tokens == nil } From a59315cc8224582f6c4c88ae77ff16ffc361e027 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:34:20 +0530 Subject: [PATCH 3/3] test(parser): re-apply dialect after Reset and feed EOF-stripped streams in sentinel corpus test [W3-02] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- pkg/sql/parser/currenttoken_corpus_test.go | 28 +++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/pkg/sql/parser/currenttoken_corpus_test.go b/pkg/sql/parser/currenttoken_corpus_test.go index 76f98fdf7..69b0b057f 100644 --- a/pkg/sql/parser/currenttoken_corpus_test.go +++ b/pkg/sql/parser/currenttoken_corpus_test.go @@ -38,17 +38,29 @@ func TestCurrentTokenSentinelUnmodifiedOverCorpus(t *testing.T) { toks = append([]models.TokenWithSpan(nil), toks...) tkz.Reset() - if tree, err := p.ParseFromModelTokens(toks); err == nil { - ast.ReleaseAST(tree) + // Tokenizer output ends with an EOF token, so the parser almost + // never walks past len(p.tokens). Also feed the stream with that + // trailing EOF stripped so currentToken/peekToken reach the + // per-parser sentinel (&p.eof) on every dialect-specific path. + streams := [][]models.TokenWithSpan{toks} + if n := len(toks); n > 0 && toks[n-1].Token.Type == models.TokenTypeEOF { + streams = append(streams, toks[:n-1]) } - p.ParseWithRecoveryFromModelTokens(toks) - if tree, err := p.ParseContextFromModelTokens(t.Context(), toks); err == nil { - ast.ReleaseAST(tree) - } - if !parser.EOFSentinelClean(p) { - t.Fatalf("dialect %s, %s#%d: EOF sentinel modified", dn, st.File, st.Index) + for _, ts := range streams { + if tree, err := p.ParseFromModelTokens(ts); err == nil { + ast.ReleaseAST(tree) + } + p.ParseWithRecoveryFromModelTokens(ts) + if tree, err := p.ParseContextFromModelTokens(t.Context(), ts); err == nil { + ast.ReleaseAST(tree) + } + if !parser.EOFSentinelClean(p) { + t.Fatalf("dialect %s, %s#%d: EOF sentinel modified", dn, st.File, st.Index) + } } p.Reset() + // Reset clears the dialect; re-apply so every statement runs under dn. + p.ApplyOptions(parser.WithDialect(dn)) if !parser.EOFSentinelClean(p) || !parser.CursorRepointed(p) { t.Fatalf("dialect %s, %s#%d: Reset left a stale cursor or dirty sentinel", dn, st.File, st.Index) }