From 015613f06cb99d7451749272a64b220df6c8263a Mon Sep 17 00:00:00 2001 From: "Marcelo E. Magallon" Date: Fri, 10 Jul 2026 16:58:43 -0600 Subject: [PATCH] fix(tokenizer): restore default dialect on pool return A Tokenizer obtained from the pool can have its dialect changed via SetDialect (e.g. by `validate --dialect mysql`). Reset did not clear the dialect or its keyword table, so returning the tokenizer to the pool left a non-default dialect in a shared instance. A later GetTokenizer caller could then receive a tokenizer configured for the wrong dialect and silently mis-parse. Restore the default (PostgreSQL) dialect in PutTokenizer, at the pool boundary. This is deliberately not done in Reset: Reset also runs at the start of every Tokenize call, where the configured dialect must persist across calls ("set dialect once, tokenize many"). SetDialect rebuilds the keyword table, so the restore only pays that cost when a non-default dialect was actually configured; the common PostgreSQL path is unaffected. Add regression tests covering the pool round-trip (no leak) and the dialect-persistence contract that Reset/Tokenize must uphold. Fixes: #524 --- CHANGELOG.md | 12 ++++ pkg/sql/tokenizer/pool.go | 23 ++++++- pkg/sql/tokenizer/pool_dialect_test.go | 92 ++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 pkg/sql/tokenizer/pool_dialect_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5810b785..b1c46d0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Tokenizer pool no longer leaks dialect state**: a tokenizer whose dialect was + changed via `SetDialect` (e.g. by `validate --dialect mysql`) is now restored to + the default (PostgreSQL) dialect when returned to the pool via `PutTokenizer`. + Previously `Reset` did not clear the dialect/keywords, so a later + `GetTokenizer` caller could receive a tokenizer configured for the wrong + dialect and silently mis-parse. The restore lives in `PutTokenizer` (the pool + boundary), not `Reset`, because `Reset` also runs at the start of every + `Tokenize` call where the configured dialect must persist. The keyword table is + rebuilt only when a non-default dialect was configured, so the common path is + unaffected. + ## [1.14.0] - 2026-04-12 — Dialect-Aware Transforms, Snowflake 100%, Schema Introspection Headline themes: dialect-aware transforms, Snowflake at 100% of the QA corpus, ClickHouse significantly expanded (83% of the QA corpus, up from 53%), live schema introspection, SQL transpilation, and first-class integration sub-modules (OpenTelemetry and GORM). Drop-in upgrade from v1.13.0 — no breaking changes. diff --git a/pkg/sql/tokenizer/pool.go b/pkg/sql/tokenizer/pool.go index 9122e472..d13d4044 100644 --- a/pkg/sql/tokenizer/pool.go +++ b/pkg/sql/tokenizer/pool.go @@ -19,6 +19,7 @@ import ( "sync" "github.com/ajitpratap0/GoSQLX/pkg/metrics" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" ) // bufferPool is used to reuse bytes.Buffer instances during tokenization. @@ -125,10 +126,22 @@ func GetTokenizer() *Tokenizer { // - Position tracking reset to initial state // - Line tracking cleared but capacity preserved // - Debug logger cleared -// - Keywords preserved (immutable configuration) +// - Dialect restored to the default (PostgreSQL) so dialect-specific keyword +// state does not leak to the next caller func PutTokenizer(t *Tokenizer) { if t != nil { t.Reset() + + // Restore the default dialect so a pooled tokenizer never leaks + // dialect-specific keyword state to a later caller. This is done here + // (at the pool boundary) rather than in Reset, which also runs at the + // start of every Tokenize call where the configured dialect must + // persist. SetDialect rebuilds the keyword table, so only pay that cost + // when a non-default dialect was configured. + if t.dialect != keywords.DialectPostgreSQL { + t.SetDialect(keywords.DialectPostgreSQL) + } + tokenizerPool.Put(t) // Record pool return @@ -153,7 +166,9 @@ func PutTokenizer(t *Tokenizer) { // - pos: Line 1, Column 0, Index 0 // - lineStarts: Empty slice with preserved capacity (contains [0]) // - input: nil (ready for new input) -// - keywords: Preserved (immutable, no need to reset) +// - dialect/keywords: preserved (Reset also runs at the start of every +// Tokenize call, so the configured dialect must persist; the pool restores +// the default dialect in PutTokenizer instead) // - logger: nil (must be set again if needed) // // Performance: By preserving slice capacity, subsequent Tokenize() calls @@ -178,7 +193,9 @@ func (t *Tokenizer) Reset() { t.line = 0 - // Don't reset keywords as they're constant + // Don't reset keywords/dialect here: Reset also runs at the start of every + // Tokenize call, where the configured dialect must persist across calls. + // Dialect is restored to the default in PutTokenizer, at the pool boundary. t.logger = nil // Preserve Comments slice capacity but reset length diff --git a/pkg/sql/tokenizer/pool_dialect_test.go b/pkg/sql/tokenizer/pool_dialect_test.go new file mode 100644 index 00000000..49c3bde5 --- /dev/null +++ b/pkg/sql/tokenizer/pool_dialect_test.go @@ -0,0 +1,92 @@ +// 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 tokenizer - pool_dialect_test.go +// Regression tests ensuring the tokenizer pool does not leak dialect-specific +// keyword state between callers. + +package tokenizer + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +// TestReset_PreservesDialect verifies that Reset preserves the configured +// dialect. Reset runs at the start of every Tokenize call, so a dialect set via +// SetDialect must survive it — otherwise "set dialect once, tokenize many" +// would break. +func TestReset_PreservesDialect(t *testing.T) { + tkz, err := New() + if err != nil { + t.Fatalf("New() error = %v", err) + } + + tkz.SetDialect(keywords.DialectMySQL) + tkz.Reset() + if tkz.Dialect() != keywords.DialectMySQL { + t.Errorf("after Reset, dialect = %q, want %q (dialect must persist across Tokenize)", + tkz.Dialect(), keywords.DialectMySQL) + } +} + +// TestTokenize_PreservesDialectAcrossCalls guards the "set dialect once, +// tokenize many" pattern end-to-end: the dialect must survive the internal +// Reset that Tokenize performs on each call. +func TestTokenize_PreservesDialectAcrossCalls(t *testing.T) { + tkz, err := NewWithDialect(keywords.DialectSQLServer) + if err != nil { + t.Fatalf("NewWithDialect error = %v", err) + } + for i := 0; i < 3; i++ { + if _, err := tkz.Tokenize([]byte("SELECT @@VERSION")); err != nil { + t.Fatalf("Tokenize #%d error = %v", i, err) + } + if tkz.Dialect() != keywords.DialectSQLServer { + t.Fatalf("after Tokenize #%d, dialect = %q, want %q", + i, tkz.Dialect(), keywords.DialectSQLServer) + } + } +} + +// TestPool_NoDialectLeak verifies that returning a dialect-configured tokenizer +// to the pool does not leak that dialect to a subsequent acquisition. +func TestPool_NoDialectLeak(t *testing.T) { + // Acquire, switch to a non-default dialect, and return to the pool. + a := GetTokenizer() + a.SetDialect(keywords.DialectMySQL) + PutTokenizer(a) + + // The next acquisition must be a clean PostgreSQL-default tokenizer. + b := GetTokenizer() + defer PutTokenizer(b) + if b.Dialect() != keywords.DialectPostgreSQL { + t.Errorf("pooled tokenizer leaked dialect: got %q, want %q", + b.Dialect(), keywords.DialectPostgreSQL) + } +} + +// TestReset_DefaultDialectStaysDefault is a sanity check that the common path +// (already the default dialect) still ends up at the default after Reset. +func TestReset_DefaultDialectStaysDefault(t *testing.T) { + tkz, err := New() + if err != nil { + t.Fatalf("New() error = %v", err) + } + tkz.Reset() + if tkz.Dialect() != keywords.DialectPostgreSQL { + t.Errorf("after Reset, dialect = %q, want %q", tkz.Dialect(), keywords.DialectPostgreSQL) + } +}