From f62197911ff3413541ec12f6648a491dbe1d7a78 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 09:57:00 +0530 Subject: [PATCH] fix(cli,mcp,transpiler): make watch honour --dialect, add parse_sql dialect, rewrite SQL Server TOP - watch: --dialect flag, validates --mode, re-validates on change, and formats in place through the same safety-checked path as format -i - mcp: parse_sql accepts a dialect; format_sql description says comments are kept - transpile: help lists every dialect; SELECT TOP becomes LIMIT/FETCH (and LIMIT becomes TOP for SQL Server); PERCENT, WITH TIES, dynamic counts and DML TOP return errors instead of changing meaning - lint: syntax errors are printed once Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 4 + cmd/gosqlx/cmd/lint.go | 5 +- cmd/gosqlx/cmd/lint_output_test.go | 49 ++++++ cmd/gosqlx/cmd/transpile.go | 11 +- cmd/gosqlx/cmd/transpile_test.go | 40 +++++ cmd/gosqlx/cmd/watch.go | 40 +++-- cmd/gosqlx/cmd/watch_cmd.go | 75 ++++++--- cmd/gosqlx/cmd/watch_test.go | 234 +++++++++++++++++++++++++++++ pkg/mcp/parse_dialect_test.go | 105 +++++++++++++ pkg/mcp/server.go | 14 +- pkg/mcp/tools.go | 19 ++- pkg/mcp/tools_internal_test.go | 8 +- pkg/transpiler/dialect_rules.go | 7 +- pkg/transpiler/row_limits.go | 112 ++++++++++++++ pkg/transpiler/row_limits_test.go | 145 ++++++++++++++++++ pkg/transpiler/transpiler.go | 10 +- 16 files changed, 820 insertions(+), 58 deletions(-) create mode 100644 cmd/gosqlx/cmd/lint_output_test.go create mode 100644 cmd/gosqlx/cmd/transpile_test.go create mode 100644 cmd/gosqlx/cmd/watch_test.go create mode 100644 pkg/mcp/parse_dialect_test.go create mode 100644 pkg/transpiler/row_limits.go create mode 100644 pkg/transpiler/row_limits_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 50360c8b..289ccffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- Watch now respects `--dialect`, re-validates changes and formats files in place + with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. +- Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite + SQL Server SELECT TOP limits across dialects, refusing PERCENT and WITH TIES. - Scan raw dialect statements and standalone VALUES for injection patterns, recover panics at every C export, and guard unlinked tokenizer scratch calls. - Formatter lower-case mode covers expression and built-in function keywords; diff --git a/cmd/gosqlx/cmd/lint.go b/cmd/gosqlx/cmd/lint.go index e037645a..02b40c4c 100644 --- a/cmd/gosqlx/cmd/lint.go +++ b/cmd/gosqlx/cmd/lint.go @@ -265,7 +265,7 @@ func lintRun(cmd *cobra.Command, args []string) error { totalViolations := errorCount + warningCount + infoCount if syntaxErrorCount > 0 { - return fmt.Errorf("%d file(s) contain SQL syntax errors (and %d violation(s))", syntaxErrorCount, totalViolations) + return silentError{fmt.Errorf("%d file(s) contain SQL syntax errors (and %d violation(s))", syntaxErrorCount, totalViolations)} } if totalViolations > 0 { return fmt.Errorf("found %d violation(s): %d error(s), %d warning(s), %d info(s)", @@ -413,7 +413,8 @@ func lintOutcomeError(violations []linter.Violation, synErr error) error { } if synErr != nil { - return fmt.Errorf("invalid SQL: %w", synErr) + // The report already includes the syntax error; main only needs the failure status. + return silentError{fmt.Errorf("invalid SQL: %w", synErr)} } if errorCount > 0 || (lintFailOnWarn && warningCount > 0) { return fmt.Errorf("%d error(s) and %d warning(s) found", errorCount, warningCount) diff --git a/cmd/gosqlx/cmd/lint_output_test.go b/cmd/gosqlx/cmd/lint_output_test.go new file mode 100644 index 00000000..c3e2bba5 --- /dev/null +++ b/cmd/gosqlx/cmd/lint_output_test.go @@ -0,0 +1,49 @@ +// 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 cmd + +import ( + "strings" + "testing" +) + +func TestLintSyntaxErrorReportedOnce(t *testing.T) { + bad := writeTemp(t, "invalid.sql", "SELECT FROM WHERE (\n") + for _, tc := range []struct { + name string + stdin *string + args []string + }{ + {"inline", nil, []string{"lint", "SELECT FROM WHERE ("}}, + {"stdin", str("SELECT FROM WHERE ("), []string{"lint"}}, + {"file", nil, []string{"lint", bad}}, + } { + t.Run(tc.name, func(t *testing.T) { + res := runCLI(t, tc.stdin, tc.args...) + if res.err == nil { + t.Fatal("invalid SQL must return an error for exit code 1") + } + if !IsSilentError(res.err) { + t.Fatalf("main would print an extra error after the lint report: %v", res.err) + } + if count := strings.Count(strings.ToLower(res.stdout), "syntax error:"); count != 1 { + t.Fatalf("expected one syntax error, got %d:\n%s", count, res.stdout) + } + if res.stderr != "" { + t.Fatalf("unexpected duplicate error output: %s", res.stderr) + } + }) + } +} diff --git a/cmd/gosqlx/cmd/transpile.go b/cmd/gosqlx/cmd/transpile.go index 349004a5..03735b25 100644 --- a/cmd/gosqlx/cmd/transpile.go +++ b/cmd/gosqlx/cmd/transpile.go @@ -30,10 +30,13 @@ var transpileCmd = &cobra.Command{ Short: "Convert SQL from one dialect to another", Long: `Transpile SQL between dialects. -Supported dialect pairs: - mysql → postgres - postgres → mysql - postgres → sqlite +Source and target dialects: + mysql, postgresql (postgres), sqlite, sqlserver (mssql), oracle, + snowflake, clickhouse, mariadb + +Any source/target combination is accepted. Dialect-specific rewrites cover a +subset of SQL features; other syntax is parsed and reformatted. Conversion +errors are returned for constructs known to be unsafe for the target dialect. SQL can be provided as a positional argument or piped via stdin. diff --git a/cmd/gosqlx/cmd/transpile_test.go b/cmd/gosqlx/cmd/transpile_test.go new file mode 100644 index 00000000..66958427 --- /dev/null +++ b/cmd/gosqlx/cmd/transpile_test.go @@ -0,0 +1,40 @@ +// 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 cmd + +import ( + "strings" + "testing" +) + +func TestTranspileHelpSupportedDialects(t *testing.T) { + res := runCLI(t, nil, "transpile", "--help") + if res.err != nil { + t.Fatal(res.err) + } + if strings.Contains(res.stdout, "Supported dialect pairs:") { + t.Fatalf("help still presents an incomplete pair list:\n%s", res.stdout) + } + for _, name := range []string{"mysql", "postgresql", "sqlite", "sqlserver", "oracle", "snowflake", "clickhouse", "mariadb"} { + t.Run(name, func(t *testing.T) { + if _, err := parseDialectFlag(name); err != nil { + t.Fatalf("documented dialect rejected: %v", err) + } + if !strings.Contains(res.stdout, name) { + t.Fatalf("help omits supported dialect %q:\n%s", name, res.stdout) + } + }) + } +} diff --git a/cmd/gosqlx/cmd/watch.go b/cmd/gosqlx/cmd/watch.go index e9ef4145..a5b42fb7 100644 --- a/cmd/gosqlx/cmd/watch.go +++ b/cmd/gosqlx/cmd/watch.go @@ -27,6 +27,8 @@ import ( "time" "github.com/fsnotify/fsnotify" + + "github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/config" ) // WatchMode represents the operation mode for file watching @@ -62,6 +64,16 @@ type FileWatcher struct { // NewFileWatcher creates a new file watcher instance func NewFileWatcher(opts WatchOptions) (*FileWatcher, error) { + if opts.Mode == "" { + opts.Mode = WatchModeValidate + } + if opts.ValidatorOpts == nil { + opts.ValidatorOpts = &ValidatorOptions{} + } + if opts.FormatterOpts == nil { + defaults := FormatterOptionsFromConfig(config.DefaultConfig(), nil, FormatterFlags{}) + opts.FormatterOpts = &defaults + } watcher, err := fsnotify.NewWatcher() if err != nil { return nil, fmt.Errorf("failed to create file watcher: %w", err) @@ -83,6 +95,7 @@ func (fw *FileWatcher) Watch(args []string) error { defer cancel() sigChan := make(chan os.Signal, 1) + defer signal.Stop(sigChan) signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) // Expand and add files/directories to watch @@ -226,21 +239,24 @@ func (fw *FileWatcher) formatFile(filename string, timestamp string) { return } - formatter := NewFormatter(fw.opts.Out, fw.opts.Err, *fw.opts.FormatterOpts) - fileResult := formatter.formatFile(filename) - - if fileResult.Error != nil { - fmt.Fprintf(fw.opts.Err, "[%s] %s %s - %v\n", - timestamp, colorRed("✗"), filename, fileResult.Error) + // Use the same formatting, verification and write path as format -i. + opts := *fw.opts.FormatterOpts + opts.InPlace = true + opts.Check = false + opts.Output = "" + formatter := NewFormatter(fw.opts.Out, fw.opts.Err, opts) + result, err := formatter.Format([]string{filename}) + if err != nil { + fmt.Fprintf(fw.opts.Err, "[%s] %s %s - %v\n", timestamp, colorRed("✗"), filename, err) return } - - if fileResult.Changed { - fmt.Fprintf(fw.opts.Out, "[%s] %s %s - Formatted\n", - timestamp, colorGreen("✓"), filename) + if result.FailedFiles > 0 { + return // Formatter.Format has already reported the error. + } + if result.FormattedFiles > 0 { + fmt.Fprintf(fw.opts.Out, "[%s] %s %s - Formatted\n", timestamp, colorGreen("✓"), filename) } else { - fmt.Fprintf(fw.opts.Out, "[%s] %s %s - No changes needed\n", - timestamp, colorCyan("→"), filename) + fmt.Fprintf(fw.opts.Out, "[%s] %s %s - No changes needed\n", timestamp, colorCyan("→"), filename) } } diff --git a/cmd/gosqlx/cmd/watch_cmd.go b/cmd/gosqlx/cmd/watch_cmd.go index 4ac29df9..c57efb8c 100644 --- a/cmd/gosqlx/cmd/watch_cmd.go +++ b/cmd/gosqlx/cmd/watch_cmd.go @@ -15,6 +15,8 @@ package cmd import ( + "fmt" + "github.com/spf13/cobra" ) @@ -38,26 +40,7 @@ Examples: return cmd.Help() } - modeStr, _ := cmd.Flags().GetString("mode") - debounce, _ := cmd.Flags().GetInt("debounce") - clear, _ := cmd.Flags().GetBool("clear") - watchVerbose, _ := cmd.Flags().GetBool("watch-verbose") - - mode := WatchModeValidate - if modeStr == "format" { - mode = WatchModeFormat - } - - opts := WatchOptions{ - Mode: mode, - DebounceMs: debounce, - ClearScreen: clear, - Verbose: watchVerbose, - Out: cmd.OutOrStdout(), - Err: cmd.ErrOrStderr(), - } - - fw, err := NewFileWatcher(opts) + fw, err := newWatcherFromCommand(cmd) if err != nil { return err } @@ -65,7 +48,59 @@ Examples: }, } +// newWatcherFromCommand keeps command options separate from the blocking watch loop. +func newWatcherFromCommand(cmd *cobra.Command) (*FileWatcher, error) { + modeStr, err := cmd.Flags().GetString("mode") + if err != nil { + return nil, err + } + debounce, err := cmd.Flags().GetInt("debounce") + if err != nil { + return nil, err + } + clear, err := cmd.Flags().GetBool("clear") + if err != nil { + return nil, err + } + watchVerbose, err := cmd.Flags().GetBool("watch-verbose") + if err != nil { + return nil, err + } + dialect, err := cmd.Flags().GetString("dialect") + if err != nil { + return nil, err + } + if err := normalizeDialectFlag(&dialect); err != nil { + return nil, err + } + + var mode WatchMode + switch modeStr { + case "validate": + mode = WatchModeValidate + case "format": + mode = WatchModeFormat + default: + return nil, fmt.Errorf("invalid --mode %q: want validate or format", modeStr) + } + fw, err := NewFileWatcher(WatchOptions{ + Mode: mode, + DebounceMs: debounce, + ClearScreen: clear, + Verbose: watchVerbose, + Out: cmd.OutOrStdout(), + Err: cmd.ErrOrStderr(), + }) + if err != nil { + return nil, err + } + fw.opts.ValidatorOpts.Dialect = dialect + fw.opts.FormatterOpts.Dialect = dialect + return fw, nil +} + func init() { + watchCmd.Flags().String("dialect", "", dialectFlagUsage("default: postgresql")) watchCmd.Flags().String("mode", "validate", "watch mode: validate or format") watchCmd.Flags().Int("debounce", 300, "debounce delay in milliseconds") watchCmd.Flags().Bool("clear", false, "clear screen before each re-run") diff --git a/cmd/gosqlx/cmd/watch_test.go b/cmd/gosqlx/cmd/watch_test.go new file mode 100644 index 00000000..c71b60e1 --- /dev/null +++ b/cmd/gosqlx/cmd/watch_test.go @@ -0,0 +1,234 @@ +// 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 cmd + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestWatchProcessesChangedFiles(t *testing.T) { + for _, mode := range []WatchMode{WatchModeValidate, WatchModeFormat} { + t.Run(string(mode), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "query.sql") + var out, stderr bytes.Buffer + fw, err := NewFileWatcher(WatchOptions{Mode: mode, Out: &out, Err: &stderr}) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := fw.Close(); err != nil { + t.Error(err) + } + }() + write := func(sql string) { + t.Helper() + if err := os.WriteFile(path, []byte(sql), 0600); err != nil { + t.Fatal(err) + } + } + write("SELECT 1") + fw.processFile(path) + out.Reset() + stderr.Reset() + changed := "-- keep this comment\nselect a from t where a=1" + write(changed) + fw.processFile(path) + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if stderr.Len() != 0 { + t.Fatalf("unexpected error: %s", &stderr) + } + if mode == WatchModeValidate { + if !strings.Contains(out.String(), " - Valid") { + t.Fatalf("missing validation: %s", &out) + } + if string(content) != changed { + t.Fatal("validation modified input") + } + } else { + if !strings.Contains(out.String(), " - Formatted") { + t.Fatalf("missing formatting: %s", &out) + } + if string(content) == changed || !strings.Contains(string(content), "SELECT") || !strings.Contains(string(content), "-- keep this comment") { + t.Fatalf("not formatted faithfully: %s", content) + } + out.Reset() + fw.processFile(path) + if !strings.Contains(out.String(), "No changes needed") { + t.Fatalf("formatting not stable: %s", &out) + } + } + out.Reset() + stderr.Reset() + invalid := "SELECT * FROM" + write(invalid) + fw.processFile(path) + content, err = os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(content) != invalid { + t.Fatalf("invalid input changed: %q", content) + } + if stderr.Len() == 0 { + t.Fatalf("missing syntax error, stdout: %s", &out) + } + }) + } +} + +func TestWatchFormatRefusesUnplaceableHint(t *testing.T) { + path := filepath.Join(t.TempDir(), "hint.sql") + original := "select a from /*+ FULL(t) */ t" + if err := os.WriteFile(path, []byte(original), 0600); err != nil { + t.Fatal(err) + } + var out, stderr bytes.Buffer + fw, err := NewFileWatcher(WatchOptions{Mode: WatchModeFormat, Out: &out, Err: &stderr}) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := fw.Close(); err != nil { + t.Error(err) + } + }() + fw.processFile(path) + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(content) != original || stderr.Len() == 0 { + t.Fatalf("content=%q stderr=%q", content, stderr.String()) + } +} + +func TestWatchRespectsDialect(t *testing.T) { + for _, mode := range []WatchMode{WatchModeValidate, WatchModeFormat} { + t.Run(string(mode), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "batch.sql") + source := "select 1\nGO\nselect 2" + if err := os.WriteFile(path, []byte(source), 0600); err != nil { + t.Fatal(err) + } + var out, stderr bytes.Buffer + fw, err := NewFileWatcher(WatchOptions{Mode: mode, Out: &out, Err: &stderr, + ValidatorOpts: &ValidatorOptions{Dialect: "sqlserver"}, + FormatterOpts: &CLIFormatterOptions{Dialect: "sqlserver", Uppercase: true, IndentSize: 2}, + }) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := fw.Close(); err != nil { + t.Error(err) + } + }() + fw.processFile(path) + if stderr.Len() != 0 { + t.Fatalf("SQL Server batch rejected: %s", &stderr) + } + if out.Len() == 0 { + t.Fatal("no processing output") + } + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(content), "\nGO\n") { + t.Fatalf("batch separator lost: %q", content) + } + }) + } +} + +func TestWatchCommandDialect(t *testing.T) { + for _, mode := range []string{"validate", "format"} { + for _, dialect := range []string{"sqlserver", "mssql", "postgresql"} { + t.Run(mode+"/"+dialect, func(t *testing.T) { + resetCLIFlags() + t.Cleanup(resetCLIFlags) + var out, stderr bytes.Buffer + watchCmd.SetOut(&out) + watchCmd.SetErr(&stderr) + t.Cleanup(func() { watchCmd.SetOut(nil); watchCmd.SetErr(nil) }) + if err := watchCmd.ParseFlags([]string{"--mode", mode, "--dialect", dialect}); err != nil { + t.Fatal(err) + } + fw, err := newWatcherFromCommand(watchCmd) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := fw.Close(); err != nil { + t.Error(err) + } + }() + path := filepath.Join(t.TempDir(), "batch.sql") + source := "select 1\nGO\nselect 2" + if err := os.WriteFile(path, []byte(source), 0600); err != nil { + t.Fatal(err) + } + fw.processFile(path) + content, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if dialect == "postgresql" { + if stderr.Len() == 0 { + t.Fatalf("PostgreSQL accepted GO script: %s", &out) + } + if string(content) != source { + t.Fatalf("rejected script changed: %q", content) + } + } else { + if stderr.Len() != 0 { + t.Fatalf("SQL Server rejected GO script: %s", &stderr) + } + if mode == "validate" && !strings.Contains(out.String(), " - Valid") { + t.Fatalf("missing validation: %s", &out) + } + if mode == "format" && (!strings.Contains(string(content), "SELECT") || !strings.Contains(string(content), "\nGO\n")) { + t.Fatalf("incorrect formatting: %q", content) + } + } + }) + } + } +} + +func TestWatchCommandRejectsUnknownDialect(t *testing.T) { + res := runCLI(t, nil, "watch", "--dialect", "invalid-dialect", t.TempDir()) + if res.err == nil || !strings.Contains(res.err.Error(), "unknown SQL dialect") { + t.Fatalf("expected dialect error, got %v", res.err) + } +} + +func TestNewWatcherFromCommandRejectsUnknownMode(t *testing.T) { + resetCLIFlags() + t.Cleanup(resetCLIFlags) + if err := watchCmd.ParseFlags([]string{"--mode", "lint"}); err != nil { + t.Fatal(err) + } + if _, err := newWatcherFromCommand(watchCmd); err == nil || !strings.Contains(err.Error(), "invalid --mode") { + t.Fatalf("expected invalid --mode error, got %v", err) + } +} diff --git a/pkg/mcp/parse_dialect_test.go b/pkg/mcp/parse_dialect_test.go new file mode 100644 index 00000000..23605756 --- /dev/null +++ b/pkg/mcp/parse_dialect_test.go @@ -0,0 +1,105 @@ +// 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 mcp + +import ( + "context" + "reflect" + "strings" + "testing" + + sqlkeywords "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestHandleParseSQL_Dialect(t *testing.T) { + for _, dialect := range sqlkeywords.DialectNames() { + t.Run(dialect, func(t *testing.T) { + res, err := handleParseSQL(context.Background(), makeReq(map[string]any{"sql": "SELECT 1", "dialect": dialect})) + if err != nil || res == nil || res.IsError { + t.Fatalf("parse failed: result=%+v, error=%v", res, err) + } + data := unmarshalResult(t, res) + if data["statement_count"] != float64(1) { + t.Fatalf("statement_count = %v, want 1", data["statement_count"]) + } + }) + } + for _, tc := range []struct { + name string + dialect any + want string + }{ + {"unknown", "not-a-dialect", "unsupported dialect"}, + {"wrong type", 5.0, "parameter 'dialect' must be a string"}, + } { + t.Run(tc.name, func(t *testing.T) { + res, err := handleParseSQL(context.Background(), makeReq(map[string]any{"sql": "SELECT 1", "dialect": tc.dialect})) + requireToolError(t, res, err, tc.want) + }) + } +} + +func TestHandleParseSQL_SQLServerBatchCount(t *testing.T) { + res, err := handleParseSQL(context.Background(), makeReq(map[string]any{"sql": "SELECT 1\nGO 3\nSELECT 2\nGO\n", "dialect": "sqlserver"})) + if err != nil || res == nil || res.IsError { + t.Fatalf("parse failed: result=%+v, error=%v", res, err) + } + data := unmarshalResult(t, res) + if data["statement_count"] != float64(2) { + t.Fatalf("statement_count = %v, want 2", data["statement_count"]) + } + got, ok := data["statement_types"].([]any) + if !ok { + t.Fatalf("statement_types has type %T", data["statement_types"]) + } + if !reflect.DeepEqual(got, []any{"*ast.SelectStatement", "*ast.SelectStatement"}) { + t.Errorf("statement_types = %v", got) + } +} + +func TestParseSQL_DialectSchema(t *testing.T) { + tool := New(DefaultConfig()).mcpSrv.GetTool("parse_sql") + if tool == nil { + t.Fatal("parse_sql tool not registered") + } + prop, ok := tool.Tool.InputSchema.Properties["dialect"].(map[string]any) + if !ok { + t.Fatalf("dialect property has type %T", tool.Tool.InputSchema.Properties["dialect"]) + } + got, ok := prop["enum"].([]string) + if !ok { + t.Fatalf("dialect enum has type %T", prop["enum"]) + } + if !reflect.DeepEqual(got, sqlkeywords.DialectNames()) { + t.Errorf("dialect enum = %v", got) + } + for _, required := range tool.Tool.InputSchema.Required { + if required == "dialect" { + t.Error("dialect must remain optional") + } + } +} + +func TestFormatSQL_CommentDescription(t *testing.T) { + tool := New(DefaultConfig()).mcpSrv.GetTool("format_sql") + if tool == nil { + t.Fatal("format_sql tool not registered") + } + for _, want := range []string{"Comments are preserved", "unplaceable optimizer hints"} { + if !strings.Contains(tool.Tool.Description, want) { + t.Errorf("description %q missing %q", tool.Tool.Description, want) + } + } +} diff --git a/pkg/mcp/server.go b/pkg/mcp/server.go index 443102e6..49a147f1 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -298,8 +298,8 @@ func newTool(name, title, description string, meta *mcp.Meta, opts ...mcp.ToolOp return mcp.NewTool(name, all...) } -// supportedDialects lists the dialect values accepted by validate_sql. It is -// derived from keywords.DialectNames, the single source of truth, and each +// supportedDialects lists the dialect values accepted by validate_sql and parse_sql. +// It is derived from keywords.DialectNames, the single source of truth, and each // one is accepted by gosqlx.ParseWithDialect. var supportedDialects = sqlkeywords.DialectNames() @@ -327,7 +327,8 @@ func (s *Server) registerTools() { s.mcpSrv.AddTool( newTool("format_sql", "Format SQL", "Use this when the user wants SQL pretty-printed or reformatted: one clause per line, "+ - "optional upper-case keywords and trailing semicolon. Comments are not kept. "+ + "optional upper-case keywords and trailing semicolon. Comments are preserved; "+ + "unplaceable optimizer hints are refused. "+ "Returns {formatted_sql, options}.", toolMeta(cfg, "Formatting SQL…", "Formatted SQL", false), mcp.WithString("sql", @@ -353,12 +354,17 @@ func (s *Server) registerTools() { s.mcpSrv.AddTool( newTool("parse_sql", "Parse SQL", "Use this when the user wants to know how SQL parses: returns an AST summary with "+ - "the statement count and the type of each statement.", + "the statement count and the type of each statement, optionally for a specific dialect. "+ + "GO batch separators are excluded from the count.", toolMeta(cfg, "Parsing SQL…", "Parsed SQL", false), mcp.WithString("sql", mcp.Required(), mcp.Description("The SQL string to parse"), ), + mcp.WithString("dialect", + mcp.Description("SQL dialect: "+strings.Join(supportedDialects, ", ")), + mcp.Enum(supportedDialects...), + ), ), handleParseSQL, ) diff --git a/pkg/mcp/tools.go b/pkg/mcp/tools.go index d0a45ba1..ae90e3e7 100644 --- a/pkg/mcp/tools.go +++ b/pkg/mcp/tools.go @@ -228,12 +228,19 @@ func uppercaseSQLKeywords(sql string) string { } // parseSQLInternal parses a SQL string and returns statement count and types. -func parseSQLInternal(sql string) (map[string]any, error) { +func parseSQLInternal(sql, dialect string) (map[string]any, error) { if sql == "" { return nil, fmt.Errorf("parameter 'sql' is required and must not be empty") } - tree, err := gosqlx.Parse(sql) + var tree *ast.AST + var err error + if dialect == "" { + tree, err = gosqlx.Parse(sql) + } else { + // ParseWithDialect validates against keywords.IsValidDialect, as validate_sql does. + tree, err = gosqlx.ParseWithDialect(sql, sqlkeywords.SQLDialect(dialect)) + } if err != nil { return nil, fmt.Errorf("parse failed: %w", err) } @@ -550,7 +557,11 @@ func handleParseSQL(ctx context.Context, req mcpmcp.CallToolRequest) (*mcpmcp.Ca if errRes != nil { return errRes, nil } - return toolOutcome(parseSQLInternal(sql)) + dialect, err := optionalString(req, "dialect", "") + if err != nil { + return mcpmcp.NewToolResultError(err.Error()), nil + } + return toolOutcome(parseSQLInternal(sql, dialect)) } // handleExtractMetadata is the MCP tool handler for "extract_metadata". @@ -599,7 +610,7 @@ func handleAnalyzeSQL(ctx context.Context, req mcpmcp.CallToolRequest) (*mcpmcp. fn func() (map[string]any, error) }{ {"validate", func() (map[string]any, error) { return validateSQLInternal(sql, "") }}, - {"parse", func() (map[string]any, error) { return parseSQLInternal(sql) }}, + {"parse", func() (map[string]any, error) { return parseSQLInternal(sql, "") }}, {"metadata", func() (map[string]any, error) { return extractMetadataInternal(sql) }}, {"security", func() (map[string]any, error) { return securityScanInternal(sql) }}, {"lint", func() (map[string]any, error) { return lintSQLInternal(sql) }}, diff --git a/pkg/mcp/tools_internal_test.go b/pkg/mcp/tools_internal_test.go index 112bd6d6..ec1e168e 100644 --- a/pkg/mcp/tools_internal_test.go +++ b/pkg/mcp/tools_internal_test.go @@ -121,14 +121,14 @@ func TestFormatSQLInternal_InvalidSQL(t *testing.T) { // --- parseSQLInternal --- func TestParseSQLInternal_EmptySQL(t *testing.T) { - _, err := parseSQLInternal("") + _, err := parseSQLInternal("", "") if err == nil { t.Fatal("expected error for empty sql") } } func TestParseSQLInternal_SingleStatement(t *testing.T) { - data, err := parseSQLInternal("SELECT id FROM users") + data, err := parseSQLInternal("SELECT id FROM users", "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -145,7 +145,7 @@ func TestParseSQLInternal_SingleStatement(t *testing.T) { } func TestParseSQLInternal_MultipleStatements(t *testing.T) { - data, err := parseSQLInternal("SELECT 1; INSERT INTO t VALUES (1); DELETE FROM t") + data, err := parseSQLInternal("SELECT 1; INSERT INTO t VALUES (1); DELETE FROM t", "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -159,7 +159,7 @@ func TestParseSQLInternal_MultipleStatements(t *testing.T) { } func TestParseSQLInternal_InvalidSQL(t *testing.T) { - _, err := parseSQLInternal(")))((( @@@ !!!") + _, err := parseSQLInternal(")))((( @@@ !!!", "") if err == nil { t.Fatal("expected parse error for invalid SQL") } diff --git a/pkg/transpiler/dialect_rules.go b/pkg/transpiler/dialect_rules.go index b79aefa6..2fb7b47a 100644 --- a/pkg/transpiler/dialect_rules.go +++ b/pkg/transpiler/dialect_rules.go @@ -50,16 +50,17 @@ func register(from, to keywords.SQLDialect, rs ...RewriteRule) { } // RulesFor returns the registered rewrite rules for a dialect pair. -// Returns nil (empty) for unregistered or same-dialect pairs. +// Cross-dialect pairs include common row-limit rules; same-dialect pairs return nil. // Exported for testing. func RulesFor(from, to keywords.SQLDialect) []RewriteRule { if from == to { return nil } rs := ruleRegistry[dialectPair{from, to}] + // Never append into the registry's backing array. + rs = append(rs[:len(rs):len(rs)], rowLimitRule(to)) if r := pipesRule(from, to); r != nil { - // Full slice expression: never append into the registry's array. - return append(rs[:len(rs):len(rs)], r) + return append(rs, r) } return rs } diff --git a/pkg/transpiler/row_limits.go b/pkg/transpiler/row_limits.go new file mode 100644 index 00000000..05b43835 --- /dev/null +++ b/pkg/transpiler/row_limits.go @@ -0,0 +1,112 @@ +// 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 transpiler + +import ( + "fmt" + "strconv" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +// rowLimitRule normalizes TOP before target rendering, including nested +// queries. Percentage, tied rows and dynamic TOP expressions are refused +// rather than given different semantics in the target dialect. +func rowLimitRule(to keywords.SQLDialect) RewriteRule { + return func(stmt ast.Statement) error { + var rewriteErr error + ast.Inspect(stmt, func(n ast.Node) bool { + if rewriteErr != nil { + return false + } + // SELECT TOP is consumed below before visiting its children. + // A remaining TOP belongs to DML, which has no portable LIMIT rewrite. + if _, ok := n.(*ast.TopClause); ok && to != keywords.DialectSQLServer { + rewriteErr = fmt.Errorf("DML TOP cannot be transpiled to %s", to) + return false + } + if set, ok := n.(*ast.SetOperation); ok && to != keywords.DialectSQLServer { + for _, right := range []bool{false, true} { + operand := set.Left + if right { + operand = set.Right + } + selectOperand, ok := operand.(*ast.SelectStatement) + if !ok || selectOperand.Top == nil { + continue + } + if to == keywords.DialectSQLite { + rewriteErr = fmt.Errorf("TOP on a set-operation operand cannot be transpiled to SQLite") + return false + } + if right { + set.RightParenthesized = true + } else { + if !set.LeftParenthesized && selectOperand.With != nil { + set.LeftWithHoisted = true + } + set.LeftParenthesized = true + } + } + } + s, ok := n.(*ast.SelectStatement) + if !ok { + return true + } + if to == keywords.DialectSQLServer && s.DialectExtras().LimitWithTies { + rewriteErr = fmt.Errorf("LIMIT WITH TIES cannot be transpiled to SQL Server") + return false + } + if s.Top != nil && to != keywords.DialectSQLServer { + top := s.Top + if top.IsPercent || top.WithTies { + rewriteErr = fmt.Errorf("TOP PERCENT or WITH TIES cannot be transpiled to %s", to) + return false + } + if s.HasLimit() || s.HasOffset() || s.Fetch != nil { + rewriteErr = fmt.Errorf("TOP combined with LIMIT, OFFSET or FETCH cannot be transpiled to %s", to) + return false + } + count, ok := top.Count.(*ast.LiteralValue) + if !ok || count == nil || (count.Type != "INTEGER" && count.Type != "int" && count.Type != "NUMBER") { + rewriteErr = fmt.Errorf("non-integer TOP count cannot be transpiled to %s", to) + return false + } + limit, err := strconv.Atoi(fmt.Sprint(count.Value)) + if err != nil || limit < 0 { + rewriteErr = fmt.Errorf("non-integer TOP count cannot be transpiled to %s", to) + return false + } + s.Limit = &limit + s.Top = nil + ast.PutLiteralValue(count) + if to == keywords.DialectGeneric || to == "" { + value := int64(limit) + s.Fetch = &ast.FetchClause{FetchType: "FIRST", FetchValue: &value} + s.Limit = nil + } + } + // With an offset, SQL Server requires OFFSET/FETCH; leave that case + // to the formatter. ORDER BY alone does not prevent use of TOP. + if to == keywords.DialectSQLServer && s.Top == nil && s.Limit != nil && !s.HasOffset() && s.Fetch == nil { + s.Top = &ast.TopClause{Count: &ast.LiteralValue{Value: *s.Limit, Type: "int"}} + s.Limit = nil + } + return true + }) + return rewriteErr + } +} diff --git a/pkg/transpiler/row_limits_test.go b/pkg/transpiler/row_limits_test.go new file mode 100644 index 00000000..625022c8 --- /dev/null +++ b/pkg/transpiler/row_limits_test.go @@ -0,0 +1,145 @@ +// 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 transpiler_test + +import ( + "strings" + "testing" + + "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/transpiler" +) + +func TestTranspileTopAcrossDialects(t *testing.T) { + for _, target := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectSnowflake, keywords.DialectClickHouse, keywords.DialectOracle, keywords.DialectGeneric} { + t.Run(string(target), func(t *testing.T) { + want := "SELECT a FROM t LIMIT 1" + if target == keywords.DialectOracle || target == keywords.DialectGeneric { + want = "SELECT a FROM t FETCH FIRST 1 ROW ONLY" + } + got, err := transpiler.Transpile("SELECT TOP (1) a FROM t", keywords.DialectSQLServer, target) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("got %q, want %q", got, want) + } + tree, err := parser.ParseWithDialect(got, target) + if err != nil { + t.Fatalf("target SQL does not parse: %v", err) + } + defer ast.ReleaseAST(tree) + }) + } +} + +func TestTranspileLimitToTopAcrossDialects(t *testing.T) { + for _, source := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectSnowflake, keywords.DialectClickHouse, keywords.DialectGeneric} { + t.Run(string(source), func(t *testing.T) { + got, err := transpiler.Transpile("SELECT a FROM t ORDER BY a LIMIT 2", source, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + if got != "SELECT TOP 2 a FROM t ORDER BY a" { + t.Fatalf("unexpected SQL: %s", got) + } + }) + } +} + +func TestTranspileTopRejectsUnsupportedSemantics(t *testing.T) { + for _, target := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectSnowflake, keywords.DialectClickHouse, keywords.DialectOracle, keywords.DialectGeneric} { + for _, sql := range []string{"SELECT TOP 10 PERCENT a FROM t", "SELECT TOP 1 WITH TIES a FROM t ORDER BY a", "SELECT TOP (@n) a FROM t", "SELECT a FROM (SELECT TOP 10 PERCENT a FROM t) AS q"} { + t.Run(string(target)+"/"+sql, func(t *testing.T) { + got, err := transpiler.Transpile(sql, keywords.DialectSQLServer, target) + if err == nil || got != "" { + t.Fatalf("got %q, %v; want refusal", got, err) + } + }) + } + } +} + +func TestTranspileNestedTop(t *testing.T) { + got, err := transpiler.Transpile("SELECT a FROM (SELECT TOP 3 a FROM t) AS q", keywords.DialectSQLServer, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + if strings.Contains(got, "TOP") || !strings.Contains(got, "LIMIT 3") { + t.Fatalf("nested TOP not rewritten: %s", got) + } +} + +func TestTranspileRowLimitEdges(t *testing.T) { + tests := []struct { + name, sql, want string + from, to keywords.SQLDialect + }{ + {"top_zero", "SELECT TOP 0 a FROM t", "SELECT a FROM t LIMIT 0", keywords.DialectSQLServer, keywords.DialectPostgreSQL}, + {"limit_zero", "SELECT a FROM t LIMIT 0", "SELECT TOP 0 a FROM t", keywords.DialectPostgreSQL, keywords.DialectSQLServer}, + {"limit_offset", "SELECT a FROM t ORDER BY a LIMIT 2 OFFSET 3", "SELECT a FROM t ORDER BY a OFFSET 3 ROWS FETCH NEXT 2 ROWS ONLY", keywords.DialectPostgreSQL, keywords.DialectSQLServer}, + {"cte", "WITH q AS (SELECT TOP 2 a FROM t) SELECT a FROM q", "WITH q AS (SELECT a FROM t LIMIT 2) SELECT a FROM q", keywords.DialectSQLServer, keywords.DialectPostgreSQL}, + {"set_operation", "SELECT TOP 2 a FROM t UNION ALL SELECT TOP 3 a FROM u", "(SELECT a FROM t LIMIT 2) UNION ALL (SELECT a FROM u LIMIT 3)", keywords.DialectSQLServer, keywords.DialectPostgreSQL}, + {"cte_set_operation", "WITH q AS (SELECT a FROM t) SELECT TOP 2 a FROM q UNION ALL SELECT a FROM q", "WITH q AS (SELECT a FROM t) (SELECT a FROM q LIMIT 2) UNION ALL SELECT a FROM q", keywords.DialectSQLServer, keywords.DialectPostgreSQL}, + {"same_dialect", "SELECT TOP 10 PERCENT a FROM t", "SELECT TOP 10 PERCENT a FROM t", keywords.DialectSQLServer, keywords.DialectSQLServer}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := transpiler.Transpile(tt.sql, tt.from, tt.to) + if err != nil { + t.Fatal(err) + } + if got != tt.want { + t.Fatalf("got %q, want %q", got, tt.want) + } + tree, err := parser.ParseWithDialect(got, tt.to) + if err != nil { + t.Fatalf("target SQL does not parse: %v", err) + } + defer ast.ReleaseAST(tree) + }) + } +} + +func TestTranspileTopSetOperandToSQLiteRefused(t *testing.T) { + got, err := transpiler.Transpile("SELECT TOP 2 a FROM t UNION ALL SELECT a FROM u", keywords.DialectSQLServer, keywords.DialectSQLite) + if err == nil || got != "" { + t.Fatalf("got %q, %v; want refusal", got, err) + } +} + +func TestTranspileLimitWithTiesToSQLServerRefused(t *testing.T) { + for _, suffix := range []string{"", " OFFSET 1"} { + t.Run(suffix, func(t *testing.T) { + got, err := transpiler.Transpile("SELECT a FROM t ORDER BY a LIMIT 2 WITH TIES"+suffix, keywords.DialectClickHouse, keywords.DialectSQLServer) + if err == nil || got != "" { + t.Fatalf("got %q, %v; want refusal", got, err) + } + }) + } +} + +func TestTranspileDMLTopRefused(t *testing.T) { + for _, sql := range []string{"UPDATE TOP (2) t SET a = 1", "DELETE TOP (2) FROM t"} { + t.Run(sql, func(t *testing.T) { + got, err := transpiler.Transpile(sql, keywords.DialectSQLServer, keywords.DialectPostgreSQL) + if err == nil || got != "" || !strings.Contains(err.Error(), "DML TOP") { + t.Fatalf("got %q, %v; want DML TOP refusal", got, err) + } + }) + } +} diff --git a/pkg/transpiler/transpiler.go b/pkg/transpiler/transpiler.go index e5f8f6ab..7a6c9a59 100644 --- a/pkg/transpiler/transpiler.go +++ b/pkg/transpiler/transpiler.go @@ -16,13 +16,13 @@ // input SQL, applying a chain of rewrite rules that mutate dialect-specific // AST constructs in place, and then reformatting the result. // -// Supported dialect pairs: +// Additional dialect-specific rewrite sets: // - MySQL → PostgreSQL // - PostgreSQL → MySQL // - PostgreSQL → SQLite // -// For unsupported dialect pairs the SQL is parsed and reformatted without any -// dialect-specific rewrites (passthrough with normalisation). +// All cross-dialect pairs apply common row-limit, operator, identifier and +// string-literal rewrites. Constructs without a safe equivalent return an error. // // Example: // @@ -51,8 +51,8 @@ type RewriteRule func(stmt ast.Statement) error // rules for the (from → to) dialect pair, and returns the reformatted SQL. // // If from == to, the SQL is parsed and reformatted with no rewrites applied. -// If no rules are registered for the pair, the SQL is returned normalised -// (parsed and reformatted without dialect-specific changes). +// All cross-dialect pairs apply common rewrites in addition to any registered +// dialect-specific rules. // SQL Server GO batch scopes cannot be represented in other dialects and // return an error rather than dropping boundaries or repeat counts. // Operators and literals whose meaning depends on the dialect (^ as power or