From 9f7d9e9d0c5e4167d109e516ac36ee87eba95608 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:27:38 +0530 Subject: [PATCH 1/4] perf(gosqlx): build wrapped error messages lazily [W1-03] The 21 fmt.Errorf("%w: %w", sentinel, err) sites in pkg/gosqlx formatted the full message on every error, although many callers only test the error with errors.Is. They now return an unexported wrapErr holding both errors in a [2]error array: Error() builds the identical "sentinel: inner" text on demand, and Unwrap() []error returns a slice of the array, so errors.Is and errors.As traverse it without allocating. Sites with extra context text keep fmt.Errorf unchanged. Tests: a compatibility suite over 242 distinct invalid inputs across every public entry point that wraps errors compares Error() text, errors.Is for every sentinel and errors.As results against the former fmt.Errorf construction, computed independently from the tokenizer and parser. M4, GOGC=off -cpu 1, 10 interleaved process pairs: Parse syntax_mid -28.1%, syntax_end -28.8%, tokenize -62.8%; Validate syntax_mid -26.9%, syntax_end -29.9%, tokenize -28.3% (all p <= 0.029); allocs e.g. 21 -> 12, 20 -> 4. long_prefix (128 identifiers before the error) -1% to -3%. Success paths unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- pkg/gosqlx/error_bench_test.go | 67 ++++++ pkg/gosqlx/errors.go | 15 ++ pkg/gosqlx/gosqlx.go | 30 +-- pkg/gosqlx/lint.go | 4 +- pkg/gosqlx/tree.go | 8 +- pkg/gosqlx/wrap_compat_test.go | 399 +++++++++++++++++++++++++++++++++ pkg/gosqlx/wrap_test.go | 83 +++++++ 7 files changed, 585 insertions(+), 21 deletions(-) create mode 100644 pkg/gosqlx/error_bench_test.go create mode 100644 pkg/gosqlx/wrap_compat_test.go create mode 100644 pkg/gosqlx/wrap_test.go diff --git a/pkg/gosqlx/error_bench_test.go b/pkg/gosqlx/error_bench_test.go new file mode 100644 index 000000000..f6a557573 --- /dev/null +++ b/pkg/gosqlx/error_bench_test.go @@ -0,0 +1,67 @@ +// 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 gosqlx + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// BenchmarkParseError measures error handling without requesting error text. +func BenchmarkParseError(b *testing.B) { + for _, tc := range []struct{ name, sql string }{ + {"syntax_mid", "SELECT id, FROM users WHERE id = 1"}, + {"syntax_end", "SELECT id FROM users WHERE"}, + {"tokenize", "SELECT 'unterminated"}, + {"long_prefix", "SELECT " + strings.Repeat("id, ", 128) + "FROM users"}, + } { + b.Run(tc.name, func(b *testing.B) { benchmarkParseAndValidate(b, tc.sql, false) }) + } +} + +func BenchmarkParseSuccess(b *testing.B) { + for _, tc := range []struct{ name, sql string }{ + {"simple", "SELECT id, name FROM users WHERE id = 1"}, + {"complex", "SELECT u.id, COUNT(o.id) FROM users u LEFT JOIN orders o ON u.id = o.user_id GROUP BY u.id ORDER BY u.id"}, + } { + b.Run(tc.name, func(b *testing.B) { benchmarkParseAndValidate(b, tc.sql, true) }) + } +} + +func benchmarkParseAndValidate(b *testing.B, sql string, valid bool) { + b.Helper() + b.Run("Parse", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + a, err := Parse(sql) + if a != nil { + ast.ReleaseAST(a) + } + if (err == nil) != valid { + b.Fatalf("unexpected parse result: %v", err) + } + } + }) + b.Run("Validate", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if err := Validate(sql); (err == nil) != valid { + b.Fatalf("unexpected validation result: %v", err) + } + } + }) +} diff --git a/pkg/gosqlx/errors.go b/pkg/gosqlx/errors.go index b9b251d87..ba06a9e8b 100644 --- a/pkg/gosqlx/errors.go +++ b/pkg/gosqlx/errors.go @@ -16,6 +16,21 @@ package gosqlx import "errors" +// wrapErr is the lazy equivalent of fmt.Errorf("%w: %w", sentinel, err). +// Both errors must be non-nil. Keeping the pair inline lets errors.Is and +// errors.As traverse it without allocating a slice on every Unwrap call. +type wrapErr struct { + errs [2]error +} + +func newWrap(sentinel, err error) *wrapErr { + return &wrapErr{errs: [2]error{sentinel, err}} +} + +func (w *wrapErr) Error() string { return w.errs[0].Error() + ": " + w.errs[1].Error() } + +func (w *wrapErr) Unwrap() []error { return w.errs[:] } + // Sentinel error values returned (wrapped) by the new ParseTree / ParseReader // entry points. Callers can test specific failure classes with errors.Is: // diff --git a/pkg/gosqlx/gosqlx.go b/pkg/gosqlx/gosqlx.go index 455a2e6af..d70e28bcc 100644 --- a/pkg/gosqlx/gosqlx.go +++ b/pkg/gosqlx/gosqlx.go @@ -116,7 +116,7 @@ func Parse(sql string) (*ast.AST, error) { // copies the tokens while preprocessing and they are not used after. tokens, err := tokscratch.String(tkz, sql) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrTokenize, err) + return nil, newWrap(ErrTokenize, err) } // Step 3: Parse to AST directly from model tokens @@ -125,7 +125,7 @@ func Parse(sql string) (*ast.AST, error) { astNode, err := p.ParseFromModelTokens(tokens) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrSyntax, err) + return nil, newWrap(ErrSyntax, err) } return astNode, nil @@ -193,7 +193,7 @@ func Parse(sql string) (*ast.AST, error) { func ParseWithContext(ctx context.Context, sql string) (*ast.AST, error) { // Check context before starting if err := ctx.Err(); err != nil { - return nil, fmt.Errorf("%w: %w", ErrTimeout, err) + return nil, newWrap(ErrTimeout, err) } // Step 1: Get tokenizer from pool @@ -204,9 +204,9 @@ func ParseWithContext(ctx context.Context, sql string) (*ast.AST, error) { tokens, err := tokscratch.StringContext(ctx, tkz, sql) if err != nil { if ctxErr := ctx.Err(); ctxErr != nil { - return nil, fmt.Errorf("%w: %w", ErrTimeout, ctxErr) + return nil, newWrap(ErrTimeout, ctxErr) } - return nil, fmt.Errorf("%w: %w", ErrTokenize, err) + return nil, newWrap(ErrTokenize, err) } // Step 3: Parse to AST with context support @@ -216,9 +216,9 @@ func ParseWithContext(ctx context.Context, sql string) (*ast.AST, error) { astNode, err := p.ParseContextFromModelTokens(ctx, tokens) if err != nil { if ctxErr := ctx.Err(); ctxErr != nil { - return nil, fmt.Errorf("%w: %w", ErrTimeout, ctxErr) + return nil, newWrap(ErrTimeout, ctxErr) } - return nil, fmt.Errorf("%w: %w", ErrSyntax, err) + return nil, newWrap(ErrSyntax, err) } return astNode, nil @@ -245,7 +245,7 @@ func ParseWithTimeout(sql string, timeout time.Duration) (*ast.AST, error) { // (e.g., parsing completed just before the deadline on fast // machines), wrap with ErrTimeout so callers can rely on errors.Is. if ctxErr := ctx.Err(); ctxErr != nil { - return nil, fmt.Errorf("%w: %w", ErrTimeout, ctxErr) + return nil, newWrap(ErrTimeout, ctxErr) } return nil, err } @@ -255,7 +255,7 @@ func ParseWithTimeout(sql string, timeout time.Duration) (*ast.AST, error) { // completes. Callers relying on errors.Is(err, ErrTimeout) must see the // timeout in that case. if ctxErr := ctx.Err(); ctxErr != nil { - return nil, fmt.Errorf("%w: %w", ErrTimeout, ctxErr) + return nil, newWrap(ErrTimeout, ctxErr) } return astNode, nil } @@ -281,7 +281,7 @@ func Validate(sql string) error { // Use the dedicated validation fast-path that avoids building a full AST err := parser.Validate(sql) if err != nil { - return fmt.Errorf("%w: %w", ErrSyntax, err) + return newWrap(ErrSyntax, err) } return nil @@ -307,7 +307,7 @@ func ParseBytes(sql []byte) (*ast.AST, error) { tokens, err := tokscratch.Bytes(tkz, sql) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrTokenize, err) + return nil, newWrap(ErrTokenize, err) } p := parser.GetParser() @@ -315,7 +315,7 @@ func ParseBytes(sql []byte) (*ast.AST, error) { astNode, err := p.ParseFromModelTokens(tokens) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrSyntax, err) + return nil, newWrap(ErrSyntax, err) } return astNode, nil @@ -732,7 +732,7 @@ func ParseWithRecovery(sql string) ([]ast.Statement, []error) { tokens, err := tokscratch.String(tkz, sql) if err != nil { - return nil, []error{fmt.Errorf("%w: %w", ErrTokenize, err)} + return nil, []error{newWrap(ErrTokenize, err)} } p := parser.GetParser() @@ -742,7 +742,7 @@ func ParseWithRecovery(sql string) ([]ast.Statement, []error) { if len(recoveryErrs) > 0 { wrapped := make([]error, len(recoveryErrs)) for i, e := range recoveryErrs { - wrapped[i] = fmt.Errorf("%w: %w", ErrSyntax, e) + wrapped[i] = newWrap(ErrSyntax, e) } return stmts, wrapped } @@ -787,7 +787,7 @@ func ParseWithDialect(sql string, dialect keywords.SQLDialect) (*ast.AST, error) } astNode, err := parser.ParseWithDialect(sql, dialect) if err != nil { - return nil, fmt.Errorf("%w: %w", ErrSyntax, err) + return nil, newWrap(ErrSyntax, err) } return astNode, nil } diff --git a/pkg/gosqlx/lint.go b/pkg/gosqlx/lint.go index 8f7934477..8c8e1187e 100644 --- a/pkg/gosqlx/lint.go +++ b/pkg/gosqlx/lint.go @@ -83,9 +83,9 @@ func lintResult(fileResult linter.FileResult) (*LintResult, error) { res := &LintResult{Violations: fileResult.Violations} switch { case fileResult.TokenizeError != nil: - return res, fmt.Errorf("%w: %w", ErrTokenize, fileResult.TokenizeError) + return res, newWrap(ErrTokenize, fileResult.TokenizeError) case fileResult.ParseError != nil: - return res, fmt.Errorf("%w: %w", ErrSyntax, fileResult.ParseError) + return res, newWrap(ErrSyntax, fileResult.ParseError) } return res, nil } diff --git a/pkg/gosqlx/tree.go b/pkg/gosqlx/tree.go index d296822de..c7e460721 100644 --- a/pkg/gosqlx/tree.go +++ b/pkg/gosqlx/tree.go @@ -329,7 +329,7 @@ func parseWithConfig(ctx context.Context, sql string, cfg parseOptions) (*ast.AS if ctxErr := ctx.Err(); ctxErr != nil { return nil, wrapContextErr(ctxErr) } - return nil, fmt.Errorf("%w: %w", ErrTokenize, tokErr) + return nil, newWrap(ErrTokenize, tokErr) } p := parser.GetParser() @@ -350,7 +350,7 @@ func parseWithConfig(ctx context.Context, sql string, cfg parseOptions) (*ast.AS // Join all diagnostics but surface them under ErrSyntax so // errors.Is(err, ErrSyntax) still matches. joined := errors.Join(errs...) - return &ast.AST{Statements: stmts}, fmt.Errorf("%w: %w", ErrSyntax, joined) + return &ast.AST{Statements: stmts}, newWrap(ErrSyntax, joined) } recovered := &ast.AST{Statements: stmts} if cfg.comments { @@ -364,7 +364,7 @@ func parseWithConfig(ctx context.Context, sql string, cfg parseOptions) (*ast.AS if ctxErr := ctx.Err(); ctxErr != nil { return nil, wrapContextErr(ctxErr) } - return nil, fmt.Errorf("%w: %w", ErrSyntax, err) + return nil, newWrap(ErrSyntax, err) } if cfg.comments { attachComments(astNode, tkz.Comments) @@ -388,7 +388,7 @@ func wrapContextErr(err error) error { if err == nil { return nil } - return fmt.Errorf("%w: %w", ErrTimeout, err) + return newWrap(ErrTimeout, err) } // ─── Format options on the new top-level API ────────────────────────────── diff --git a/pkg/gosqlx/wrap_compat_test.go b/pkg/gosqlx/wrap_compat_test.go new file mode 100644 index 000000000..829b8b70f --- /dev/null +++ b/pkg/gosqlx/wrap_compat_test.go @@ -0,0 +1,399 @@ +// 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 gosqlx + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + "time" + + sqlerrors "github.com/ajitpratap0/GoSQLX/pkg/errors" + "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" +) + +// These references call the lower-level APIs and construct the former +// fmt.Errorf errors. They never inspect the new wrapper's fields, so a changed +// sentinel, missing wrapper, or changed prefix cannot bless itself. +// legacyParseError returns the inner errors and the error the eager wrapping produced. +func legacyParseError(sql string, dialect keywords.SQLDialect, mode string) ([]error, error) { + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + if dialect != "" { + tkz.SetDialect(dialect) + } + tokens, err := tkz.TokenizeString(sql) + if mode == "context" { + tokens, err = tkz.TokenizeStringContext(context.Background(), sql) + } + if err != nil { + return []error{err}, fmt.Errorf("%w: %w", ErrTokenize, err) + } + p := parser.GetParser() + defer parser.PutParser(p) + if dialect != "" { + p.ApplyOptions(parser.WithDialect(string(dialect))) + } + if mode == "recovery" { + stmts, errs := p.ParseWithRecoveryFromModelTokens(tokens) + ast.ReleaseAST(&ast.AST{Statements: stmts}) + if len(errs) == 0 { + return nil, nil + } + return errs, fmt.Errorf("%w: %w", ErrSyntax, errors.Join(errs...)) + } + var a *ast.AST + if mode == "context" { + a, err = p.ParseContextFromModelTokens(context.Background(), tokens) + } else { + a, err = p.ParseFromModelTokens(tokens) + } + if a != nil { + ast.ReleaseAST(a) + } + if err == nil { + return nil, nil + } + return []error{err}, fmt.Errorf("%w: %w", ErrSyntax, err) +} + +func legacyDialectError(sql string, dialect keywords.SQLDialect) error { + a, err := parser.ParseWithDialect(sql, dialect) + if a != nil { + ast.ReleaseAST(a) + } + if err != nil { + return fmt.Errorf("%w: %w", ErrSyntax, err) + } + return nil +} + +func legacyValidateError(sql string) error { + if strings.TrimSpace(sql) == "" { + return fmt.Errorf("%w: empty input", ErrSyntax) + } + if err := parser.Validate(sql); err != nil { + return fmt.Errorf("%w: %w", ErrSyntax, err) + } + return nil +} + +func legacyLintError(sql string, dialect keywords.SQLDialect) error { + l := defaultLinter() + if dialect != "" { + l = linterForDialect(dialect) + } + r := l.LintString(sql, "") + if r.Error != nil { + return r.Error + } + if r.TokenizeError != nil { + return fmt.Errorf("%w: %w", ErrTokenize, r.TokenizeError) + } + if r.ParseError != nil { + return fmt.Errorf("%w: %w", ErrSyntax, r.ParseError) + } + return nil +} + +func legacyNormalizeError(sql string) error { + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + tokens, err := tkz.TokenizeString(sql) + if err != nil { + return fmt.Errorf("fingerprint: tokenization failed: %w", err) + } + p := parser.GetParser() + defer parser.PutParser(p) + a, err := p.ParseFromModelTokens(tokens) + if a != nil { + ast.ReleaseAST(a) + } + if err != nil { + return fmt.Errorf("fingerprint: parsing failed: %w", err) + } + return nil +} + +func assertLegacyError(t *testing.T, got, want error) { + t.Helper() + if (got == nil) != (want == nil) { + t.Fatalf("error presence: got %v, want %v", got, want) + } + if want == nil { + return + } + if got.Error() != want.Error() { + t.Fatalf("error text:\ngot %q\nwant %q", got.Error(), want.Error()) + } + for _, sentinel := range []error{ErrTokenize, ErrSyntax, ErrTimeout, ErrUnsupportedDialect, ErrTooLarge, context.Canceled, context.DeadlineExceeded} { + if errors.Is(got, sentinel) != errors.Is(want, sentinel) { + t.Errorf("errors.Is(%v): got %v, want %v", sentinel, errors.Is(got, sentinel), errors.Is(want, sentinel)) + } + } + var gotStructured, wantStructured *sqlerrors.Error + gotAs, wantAs := errors.As(got, &gotStructured), errors.As(want, &wantStructured) + if gotAs != wantAs { + t.Fatalf("structured error presence: got %v, want %v", gotAs, wantAs) + } + if wantAs && (gotStructured.Code != wantStructured.Code || gotStructured.Location != wantStructured.Location) { + t.Errorf("structured code/location: got %s/%+v, want %s/%+v", gotStructured.Code, gotStructured.Location, wantStructured.Code, wantStructured.Location) + } +} + +func TestWrapErrLegacyCompatibilityCorpus(t *testing.T) { + invalid := []string{"", " \t\n"} + templates := []string{ + "SELECT * FROM", "SELECT (", "SELECT 1 FROM t WHERE", "INSERT INTO t VALUES (", + "UPDATE t SET", "DELETE FROM", "SELECT 'unterminated", "SELECT \x00", "SELECT * FROM t WHERE x =", "SELECT 1 +", + } + for i := 0; i < 24; i++ { + for _, sql := range templates { + invalid = append(invalid, strings.Repeat("\n", i/6)+strings.Repeat(" ", i%6)+sql) + } + } + if len(invalid) < 200 { + t.Fatal("compatibility corpus requires at least 200 invalid inputs") + } + seen := make(map[string]bool, len(invalid)) + for _, sql := range invalid { + if seen[sql] { + t.Fatalf("duplicate corpus input: %q", sql) + } + seen[sql] = true + } + ctx := context.Background() + for i, sql := range invalid { + t.Run(fmt.Sprintf("input_%03d", i), func(t *testing.T) { + _, parseWant := legacyParseError(sql, "", "") + _, contextWant := legacyParseError(sql, "", "context") + if parseWant == nil { + t.Fatalf("corpus input was valid: %q", sql) + } + type entrypoint struct { + name string + run func() error + want error + } + tests := []entrypoint{ + {"MustParse", func() (err error) { + defer func() { + recovered := recover() + var ok bool + err, ok = recovered.(error) + if !ok { + t.Fatalf("MustParse panic: got %v, want error", recovered) + } + }() + MustParse(sql) + return nil + }, fmt.Errorf("gosqlx.MustParse: %w", parseWant)}, + {"Parse", func() error { + a, e := Parse(sql) + if a != nil { + ast.ReleaseAST(a) + } + return e + }, parseWant}, + {"ParseBytes", func() error { + a, e := ParseBytes([]byte(sql)) + if a != nil { + ast.ReleaseAST(a) + } + return e + }, parseWant}, + {"ParseWithContext", func() error { + a, e := ParseWithContext(ctx, sql) + if a != nil { + ast.ReleaseAST(a) + } + return e + }, contextWant}, + {"ParseWithTimeout", func() error { + a, e := ParseWithTimeout(sql, time.Hour) + if a != nil { + ast.ReleaseAST(a) + } + return e + }, contextWant}, + {"ParseTree", func() error { + tr, e := ParseTree(ctx, sql) + if tr != nil { + ast.ReleaseAST(tr.Raw()) + } + return e + }, contextWant}, + {"ParseReader", func() error { + tr, e := ParseReader(ctx, strings.NewReader(sql)) + if tr != nil { + ast.ReleaseAST(tr.Raw()) + } + return e + }, contextWant}, + {"Validate", func() error { return Validate(sql) }, legacyValidateError(sql)}, + {"Format", func() error { _, e := Format(sql, DefaultFormatOptions()); return e }, fmt.Errorf("cannot format invalid SQL: %w", parseWant)}, + {"Lint", func() error { _, e := Lint(sql); return e }, legacyLintError(sql, "")}, + {"Normalize", func() error { _, e := Normalize(sql); return e }, legacyNormalizeError(sql)}, + {"Fingerprint", func() error { _, e := Fingerprint(sql); return e }, legacyNormalizeError(sql)}, + } + // Batch APIs retain their zero-based query index and extra context text. + batchWant := fmt.Errorf("query %d: %w", 1, parseWant) + tests = append(tests, + entrypoint{"ParseMultiple", func() error { aa, e := ParseMultiple([]string{"SELECT 1", sql}); releaseASTs(aa); return e }, batchWant}, + entrypoint{"ValidateMultiple", func() error { return ValidateMultiple([]string{"SELECT 1", sql}) }, batchWant}, + ) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { assertLegacyError(t, tc.run(), tc.want) }) + } + + recoveryInner, recoveryWant := legacyParseError(sql, "", "recovery") + t.Run("ParseTreeRecovery", func(t *testing.T) { _, e := ParseTree(ctx, sql, WithRecovery()); assertLegacyError(t, e, recoveryWant) }) + t.Run("ParseReaderRecovery", func(t *testing.T) { + _, e := ParseReader(ctx, strings.NewReader(sql), WithRecovery()) + assertLegacyError(t, e, recoveryWant) + }) + t.Run("ParseWithRecovery", func(t *testing.T) { + ss, got := ParseWithRecovery(sql) + ast.ReleaseAST(&ast.AST{Statements: ss}) + if len(got) != len(recoveryInner) { + t.Fatalf("diagnostic count: got %d, want %d", len(got), len(recoveryInner)) + } + sentinel := ErrSyntax + if errors.Is(recoveryWant, ErrTokenize) { + sentinel = ErrTokenize + } + for j := range got { + assertLegacyError(t, got[j], fmt.Errorf("%w: %w", sentinel, recoveryInner[j])) + } + }) + // The splitter skips blanks and trims source-position whitespace. + t.Run("ParseReaderMultiple", func(t *testing.T) { + var want error + for j, segment := range SplitStatements("SELECT 1;"+sql, "") { + segment = strings.TrimSpace(segment) + if segment == "" || commentsOnly(segment) { + continue + } + _, inner := legacyParseError(segment, "", "context") + if inner != nil { + want = fmt.Errorf("statement %d: %w", j, inner) + break + } + } + ts, got := ParseReaderMultiple(ctx, strings.NewReader("SELECT 1;"+sql)) + for _, tr := range ts { + ast.ReleaseAST(tr.Raw()) + } + assertLegacyError(t, got, want) + }) + for _, dialect := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectSQLServer} { + t.Run(string(dialect), func(t *testing.T) { + dialectWant := legacyDialectError(sql, dialect) + _, formatWant := legacyParseError(sql, dialect, "") + _, treeWant := legacyParseError(sql, dialect, "context") + t.Run("ParseWithDialect", func(t *testing.T) { + a, e := ParseWithDialect(sql, dialect) + if a != nil { + ast.ReleaseAST(a) + } + assertLegacyError(t, e, dialectWant) + }) + t.Run("ParseTree", func(t *testing.T) { + _, e := ParseTree(ctx, sql, WithDialect(string(dialect))) + assertLegacyError(t, e, treeWant) + }) + t.Run("FormatWithDialect", func(t *testing.T) { + _, e := FormatWithDialect(sql, DefaultFormatOptions(), dialect) + assertLegacyError(t, e, fmt.Errorf("cannot format invalid SQL: %w", formatWant)) + }) + t.Run("LintWithDialect", func(t *testing.T) { + _, e := LintWithDialect(sql, dialect) + assertLegacyError(t, e, legacyLintError(sql, dialect)) + }) + t.Run("Transpile", func(t *testing.T) { + a, inner := parser.ParseWithDialect(sql, dialect) + if a != nil { + ast.ReleaseAST(a) + } + var want error + if inner != nil { + want = fmt.Errorf("parse: %w", inner) + } + _, got := Transpile(sql, dialect, keywords.DialectSQLite) + assertLegacyError(t, got, want) + }) + }) + } + }) + } +} + +func TestWrapErrLegacySentinelEdges(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + deadline, cancelDeadline := context.WithDeadline(context.Background(), time.Unix(0, 0)) + defer cancelDeadline() + for _, expired := range []context.Context{ctx, deadline} { + want := fmt.Errorf("%w: %w", ErrTimeout, expired.Err()) + _, got := ParseWithContext(expired, "SELECT 1") + assertLegacyError(t, got, want) + _, got = ParseTree(expired, "SELECT 1") + assertLegacyError(t, got, want) + _, got = ParseReader(expired, strings.NewReader("SELECT 1")) + assertLegacyError(t, got, want) + _, got = ParseReaderMultiple(expired, strings.NewReader("SELECT 1")) + assertLegacyError(t, got, want) + } + _, got := ParseWithTimeout("SELECT 1", 0) + assertLegacyError(t, got, fmt.Errorf("%w: %w", ErrTimeout, context.DeadlineExceeded)) + badDialect := keywords.SQLDialect("unsupported-test-dialect") + want := fmt.Errorf("%w: %q", ErrUnsupportedDialect, badDialect) + _, got = ParseWithDialect("SELECT 1", badDialect) + assertLegacyError(t, got, want) + _, got = FormatWithDialect("SELECT 1", DefaultFormatOptions(), badDialect) + assertLegacyError(t, got, want) + _, got = LintWithDialect("SELECT 1", badDialect) + assertLegacyError(t, got, want) + _, got = ParseTree(context.Background(), "SELECT 1", WithDialect(string(badDialect))) + assertLegacyError(t, got, want) + _, got = ParseReader(context.Background(), strings.NewReader("SELECT 1"), WithMaxBytes(3)) + assertLegacyError(t, got, fmt.Errorf("%w: read %d bytes, cap is %d", ErrTooLarge, 4, 3)) + _, got = ParseReader(context.Background(), nil) + assertLegacyError(t, got, fmt.Errorf("%w: nil reader", ErrTokenize)) +} + +func TestWrapErrErrorsIsAllocs(t *testing.T) { + inner := &sqlerrors.Error{Code: sqlerrors.ErrCodeUnexpectedToken, Message: "unexpected token"} + wrapped := newWrap(ErrSyntax, inner) + // Matching only the first sentinel would not exercise inner traversal or + // the complete multi-error slice. Test hits on both branches and a miss. + for _, target := range []error{ErrSyntax, inner, ErrTokenize} { + if allocs := testing.AllocsPerRun(1000, func() { _ = errors.Is(wrapped, target) }); allocs != 0 { + t.Errorf("errors.Is(%v) allocated %g times; want 0", target, allocs) + } + } + var found *sqlerrors.Error + if allocs := testing.AllocsPerRun(1000, func() { _ = errors.As(wrapped, &found) }); allocs != 0 { + t.Errorf("errors.As allocated %g times; want 0", allocs) + } + if found != inner { + t.Fatal("errors.As did not preserve the structured error pointer") + } +} diff --git a/pkg/gosqlx/wrap_test.go b/pkg/gosqlx/wrap_test.go new file mode 100644 index 000000000..167231e4b --- /dev/null +++ b/pkg/gosqlx/wrap_test.go @@ -0,0 +1,83 @@ +// 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 gosqlx + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +type countedWrapError struct { + calls int + text string +} + +func (e *countedWrapError) Error() string { e.calls++; return e.text } + +func TestWrapErrDefersFormatting(t *testing.T) { + outer := &countedWrapError{text: "outer"} + inner := &countedWrapError{text: "inner"} + w := newWrap(outer, inner) + if !errors.Is(w, outer) || !errors.Is(w, inner) { + t.Fatal("wrapper lost an error") + } + var found *countedWrapError + if !errors.As(w, &found) || found != outer { + t.Fatal("wrapper changed errors.As traversal order") + } + if outer.calls != 0 || inner.calls != 0 { + t.Fatal("constructing or inspecting the wrapper formatted its errors") + } + got := w.Error() + if outer.calls == 0 || inner.calls == 0 { + t.Fatal("Error did not format both errors") + } + want := fmt.Errorf("%w: %w", outer, inner).Error() + if got != want || w.Error() != want { + t.Fatalf("error text: got %q, want %q", got, want) + } +} + +func TestWrapErrDialectSpecificCompatibility(t *testing.T) { + for _, tc := range []struct { + dialect keywords.SQLDialect + sql string + }{ + {keywords.DialectPostgreSQL, "SELECT $tag$unterminated"}, + {keywords.DialectMySQL, "SELECT `unterminated"}, + {keywords.DialectSQLServer, "SELECT [unterminated"}, + } { + t.Run(string(tc.dialect), func(t *testing.T) { + want := legacyDialectError(tc.sql, tc.dialect) + if want == nil { + t.Fatalf("dialect fixture must be invalid: %q", tc.sql) + } + _, got := ParseWithDialect(tc.sql, tc.dialect) + assertLegacyError(t, got, want) + _, treeWant := legacyParseError(tc.sql, tc.dialect, "context") + _, got = ParseTree(context.Background(), tc.sql, WithDialect(string(tc.dialect))) + assertLegacyError(t, got, treeWant) + _, formatWant := legacyParseError(tc.sql, tc.dialect, "") + _, got = FormatWithDialect(tc.sql, DefaultFormatOptions(), tc.dialect) + assertLegacyError(t, got, fmt.Errorf("cannot format invalid SQL: %w", formatWant)) + _, got = LintWithDialect(tc.sql, tc.dialect) + assertLegacyError(t, got, legacyLintError(tc.sql, tc.dialect)) + }) + } +} From f953bbf37f6dd613585f3b5cb1b9b5cd7c4aa52a Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:06:36 +0530 Subject: [PATCH 2/4] perf(gosqlx): drop duplicate error benchmarks, add changelog [W1-03] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 3 ++ pkg/gosqlx/error_bench_test.go | 67 ---------------------------------- 2 files changed, 3 insertions(+), 67 deletions(-) delete mode 100644 pkg/gosqlx/error_bench_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 84e85635b..5f13561bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Performance +- **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. [W1-03] + ### Fixed - **AST**: `AlterTableStatement.SQL()` now separates multiple ALTER TABLE actions with commas, matching the compact formatter and preserving action boundaries when re-parsed. - **Playground**: the GoSQLX WebAssembly module now runs in a Web Worker, so diff --git a/pkg/gosqlx/error_bench_test.go b/pkg/gosqlx/error_bench_test.go deleted file mode 100644 index f6a557573..000000000 --- a/pkg/gosqlx/error_bench_test.go +++ /dev/null @@ -1,67 +0,0 @@ -// 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 gosqlx - -import ( - "strings" - "testing" - - "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" -) - -// BenchmarkParseError measures error handling without requesting error text. -func BenchmarkParseError(b *testing.B) { - for _, tc := range []struct{ name, sql string }{ - {"syntax_mid", "SELECT id, FROM users WHERE id = 1"}, - {"syntax_end", "SELECT id FROM users WHERE"}, - {"tokenize", "SELECT 'unterminated"}, - {"long_prefix", "SELECT " + strings.Repeat("id, ", 128) + "FROM users"}, - } { - b.Run(tc.name, func(b *testing.B) { benchmarkParseAndValidate(b, tc.sql, false) }) - } -} - -func BenchmarkParseSuccess(b *testing.B) { - for _, tc := range []struct{ name, sql string }{ - {"simple", "SELECT id, name FROM users WHERE id = 1"}, - {"complex", "SELECT u.id, COUNT(o.id) FROM users u LEFT JOIN orders o ON u.id = o.user_id GROUP BY u.id ORDER BY u.id"}, - } { - b.Run(tc.name, func(b *testing.B) { benchmarkParseAndValidate(b, tc.sql, true) }) - } -} - -func benchmarkParseAndValidate(b *testing.B, sql string, valid bool) { - b.Helper() - b.Run("Parse", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - a, err := Parse(sql) - if a != nil { - ast.ReleaseAST(a) - } - if (err == nil) != valid { - b.Fatalf("unexpected parse result: %v", err) - } - } - }) - b.Run("Validate", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - if err := Validate(sql); (err == nil) != valid { - b.Fatalf("unexpected validation result: %v", err) - } - } - }) -} From f6dbc2e6f5860653eaa45d0f13bd4fb4869d0420 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:32:06 +0530 Subject: [PATCH 3/4] test(gosqlx): pin ErrTimeout on mid-flight cancel; disclose materialized-message gain [W1-03] Add a flipping-context test that kills the two surviving mutants in ParseWithContext (mid-tokenize and mid-parse cancel paths). CHANGELOG now states the gain applies to callers that drop the message, gives the materialized-message numbers, and notes Error() reflects later mutation of the inner error. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 2 +- pkg/gosqlx/wrap_ctx_test.go | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 pkg/gosqlx/wrap_ctx_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f13561bb..a3f5b9a62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Performance -- **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. [W1-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] ### 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/gosqlx/wrap_ctx_test.go b/pkg/gosqlx/wrap_ctx_test.go new file mode 100644 index 000000000..30d893890 --- /dev/null +++ b/pkg/gosqlx/wrap_ctx_test.go @@ -0,0 +1,53 @@ +package gosqlx + +import ( + "context" + "errors" + "sync/atomic" + "testing" +) + +// flipCtx is a context whose Err() returns nil for the first okCalls calls and +// context.Canceled afterwards. It lets tests reach the mid-tokenize and +// mid-parse cancellation branches of ParseWithContext deterministically. +type flipCtx struct { + context.Context + calls atomic.Int64 + okCalls int64 +} + +func (c *flipCtx) Err() error { + if c.calls.Add(1) > c.okCalls { + return context.Canceled + } + return nil +} + +// TestParseWithContext_MidFlightCancelWrapsTimeout sweeps the cancellation +// point across every ctx.Err() call and requires that every failure caused by +// cancellation wraps ErrTimeout together with context.Canceled, never +// ErrTokenize or ErrSyntax. +func TestParseWithContext_MidFlightCancelWrapsTimeout(t *testing.T) { + const sql = "SELECT a, b FROM t WHERE a IN (SELECT x FROM u) ORDER BY b" + sawTimeout := 0 + for k := int64(1); k < 400; k++ { + ctx := &flipCtx{Context: context.Background(), okCalls: k} + _, err := ParseWithContext(ctx, sql) + if err == nil { + continue + } + if !errors.Is(err, context.Canceled) { + t.Fatalf("okCalls=%d: unexpected non-cancel error: %v", k, err) + } + if !errors.Is(err, ErrTimeout) { + t.Fatalf("okCalls=%d: cancel error does not wrap ErrTimeout: %v", k, err) + } + if errors.Is(err, ErrTokenize) || errors.Is(err, ErrSyntax) { + t.Fatalf("okCalls=%d: cancel error wraps wrong sentinel: %v", k, err) + } + sawTimeout++ + } + if sawTimeout == 0 { + t.Fatal("never observed a mid-flight cancellation") + } +} From 04b78ae4edca87515785deeb340518480a7439ea Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh <18012955+ajitpratap0@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:39:03 +0530 Subject: [PATCH 4/4] style(gosqlx): add license header to wrap_ctx_test.go [W1-03] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- pkg/gosqlx/wrap_ctx_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/gosqlx/wrap_ctx_test.go b/pkg/gosqlx/wrap_ctx_test.go index 30d893890..59eac2a1c 100644 --- a/pkg/gosqlx/wrap_ctx_test.go +++ b/pkg/gosqlx/wrap_ctx_test.go @@ -1,3 +1,17 @@ +// 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 gosqlx import (