diff --git a/CHANGELOG.md b/CHANGELOG.md index 06b4c3d77..c6be899ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- **CLI security scan**: `gosqlx analyze --security` and `gosqlx lint`'s security pass now run the string scanner (`ScanSQL`) as well as the AST scan and merge the findings, so `SELECT 1; DROP TABLE users` is reported as CRITICAL instead of clean. Inline SQL that contains `/` (for example `UNION/**/SELECT` or `4 / 2`) is no longer mistaken for a missing file path. +- **Transpile row limits**: for SQL Server, `LIMIT n OFFSET m` and bare `OFFSET` without ORDER BY add `ORDER BY (SELECT NULL)`, and `FETCH FIRST` gains the required `OFFSET 0 ROWS`. Row limits inside derived tables, JOIN operands and scalar subqueries are now rewritten for the target dialect (Oracle kept `LIMIT` there). `WITH TIES` survives set-operation tails, and targets that cannot express it return an error instead of dropping it. +- **LSP**: requests for unknown methods are answered with `-32601` (MethodNotFound) instead of `-32603`. - **Formatter fidelity**: Oracle SQL*Plus `/` execute lines are kept by FormatAST, `Formatter.Format`, `FormatWithDialect` and `gosqlx format` (they were dropped, and the CLI refused such scripts). Statement counts do not change: the AST records which statements a `/` follows in `AST.ExecuteSlash`. PostgreSQL `U&"..."` identifiers keep their original spelling when formatting for PostgreSQL or no target (other targets get the decoded name; AST name fields stay decoded), so names with escaped control characters no longer produce output that fails to re-tokenize. Comments next to SQL Server `GO` lines and trailing comments of statements written without `;` stay where they were, and `GO 2 /* c */` keeps both count and comment on every entry point. - **Formatter performance**: lower-case keyword mode streams into a single builder instead of copying and re-rendering subtrees; it went from 3.5–10x to 1.2–1.5x the cost of upper-case output, with 5x to 500x fewer allocations. - **Security (`ScanSQL`)**: closed several ways to slip injection past the string scanner (also used by MCP, the C binding and WASM). Dollar-quote stripping no longer runs inside quoted strings or identifiers (`SELECT '$x$'; DROP TABLE users; SELECT '$x$'` was clean). Tautology and comment checks now cover the whole input instead of the first 10,000 bytes; inputs above 10 MiB get a CRITICAL `INPUT_TOO_LARGE` finding whatever the severity filter. Comments between keywords (`UNION/**/SELECT`, `SLEEP/**/(5)`, `;--\nDROP`, `#` comments, `/*/`, MySQL `1--1`) are matched through comment-free views of the text under each quoting and escaping convention, and MySQL/MariaDB `/*! */` and `/*M! */` bodies are scanned. The AST UNION NULL-padding check now matches the parser's NULL literal. The tautology check no longer allocates per candidate (1 MiB of `1=2` went from 50 MB to 1 MB). diff --git a/cmd/gosqlx/cmd/analyze_security_regression_test.go b/cmd/gosqlx/cmd/analyze_security_regression_test.go new file mode 100644 index 000000000..72508bbe1 --- /dev/null +++ b/cmd/gosqlx/cmd/analyze_security_regression_test.go @@ -0,0 +1,116 @@ +// 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" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/security" +) + +func TestAnalyzeSecurityRawSQLFindings(t *testing.T) { + for _, sql := range []string{"SELECT 1; DROP TABLE users", "SELECT '$x$'; DROP TABLE users; SELECT '$x$'"} { + t.Run(sql, func(t *testing.T) { + var out, errOut bytes.Buffer + analyzer := NewAnalyzer(&out, &errOut, CLIAnalyzerOptions{Security: true, Format: "json"}) + result, err := analyzer.Analyze(sql) + if err != nil { + t.Fatal(err) + } + if result.Report.CriticalIssues == 0 || result.Report.SecurityScore >= 100 { + t.Fatalf("missing critical finding/score: %+v", result.Report) + } + if err := analyzer.DisplayReport(result.Report); err != nil { + t.Fatal(err) + } + var report AnalysisReport + if err := json.Unmarshal(out.Bytes(), &report); err != nil { + t.Fatal(err) + } + found := false + for _, issue := range report.Issues { + if issue.ID == string(security.PatternStackedQuery) && issue.Severity == IssueSeverityCritical { + found = true + } + } + if !found { + t.Fatalf("missing stacked query in JSON: %s", out.String()) + } + out.Reset() + analyzer.Opts.Format = "table" + if err := analyzer.DisplayReport(result.Report); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "CRITICAL") { + t.Fatalf("missing critical finding in text: %s", out.String()) + } + }) + } + var out bytes.Buffer + analyzer := NewAnalyzer(&out, &out, CLIAnalyzerOptions{Security: true}) + result, err := analyzer.Analyze("SELECT id FROM users WHERE id = 1") + if err != nil { + t.Fatal(err) + } + if issues := filterIssuesByCategory(result.Report.Issues, IssueCategorySecurity); len(issues) != 0 { + t.Fatalf("clean SQL security issues: %+v", issues) + } + if _, err := analyzer.Analyze("SELECT 1 UNION/**/SELECT 2"); err != nil { + t.Fatalf("inline SQL treated as path: %v", err) + } +} + +func TestLintSecurityRawSQLFindings(t *testing.T) { + for _, sql := range []string{"SELECT 1; DROP TABLE users", "SELECT '$x$'; DROP TABLE users; SELECT '$x$'"} { + var out bytes.Buffer + if count := runSecurityScan(sql, "inline", &out); count == 0 || !strings.Contains(out.String(), "[CRITICAL]") || !strings.Contains(out.String(), string(security.PatternStackedQuery)) { + t.Fatalf("missing critical finding: %d %s", count, out.String()) + } + } + var out bytes.Buffer + if count := runSecurityScan("SELECT id FROM users WHERE id = 1", "inline", &out); count != 0 { + t.Fatalf("clean SQL findings: %d %s", count, out.String()) + } +} + +func TestMergeSecurityFindingsDeduplicatesAndCounts(t *testing.T) { + critical := security.Finding{Pattern: security.PatternStackedQuery, Severity: security.SeverityCritical, Description: "stacked"} + high := security.Finding{Pattern: security.PatternUnionGeneric, Severity: security.SeverityHigh, Description: "union"} + result := mergeSecurityFindings(&security.ScanResult{Findings: []security.Finding{critical}}, &security.ScanResult{Findings: []security.Finding{critical, high}}) + if len(result.Findings) != 2 || result.TotalCount != 2 || result.CriticalCount != 1 || result.HighCount != 1 { + t.Fatalf("incorrect merged summary: %+v", result) + } +} + +func TestAnalyzeSecurityExistingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "SELECT query.sql") + if err := os.WriteFile(path, []byte("SELECT 1; DROP TABLE users"), 0600); err != nil { + t.Fatal(err) + } + var out bytes.Buffer + analyzer := NewAnalyzer(&out, &out, CLIAnalyzerOptions{Security: true}) + result, err := analyzer.Analyze(path) + if err != nil { + t.Fatal(err) + } + if result.Report.CriticalIssues == 0 { + t.Fatalf("file contents not scanned: %+v", result.Report) + } +} diff --git a/cmd/gosqlx/cmd/input_utils.go b/cmd/gosqlx/cmd/input_utils.go index 08f8d7a9c..8b601c650 100644 --- a/cmd/gosqlx/cmd/input_utils.go +++ b/cmd/gosqlx/cmd/input_utils.go @@ -72,9 +72,8 @@ type InputResult struct { // Detection logic: // 1. Check if input is a valid file path (os.Stat succeeds) // 2. If file exists, validate security and read content -// 3. If not a file, check if it looks like a file path (.sql extension, path separators) -// 4. If looks like file path, return file not found error -// 5. Otherwise treat as direct SQL query +// 3. If not a file, accept input that looks like SQL +// 4. Otherwise report missing file paths or invalid input // // Security measures: // - File path validation (path traversal prevention) @@ -140,9 +139,9 @@ func DetectAndReadInput(input string) (*InputResult, error) { }, nil } - // If stat failed, check if it looks like a file path that doesn't exist - // (contains path separators or has .sql extension) - if strings.Contains(input, string(filepath.Separator)) || strings.HasSuffix(strings.ToLower(input), ".sql") { + // SQL can contain path separators in comments, division, or string literals. + // Only report a missing path when the input does not look like SQL. + if !looksLikeSQL(input) && (strings.Contains(input, string(filepath.Separator)) || strings.HasSuffix(strings.ToLower(input), ".sql")) { // Looks like a file path but doesn't exist - return the original stat error return nil, fmt.Errorf("invalid file path: %w", statErr) } diff --git a/cmd/gosqlx/cmd/lint.go b/cmd/gosqlx/cmd/lint.go index 02b40c4c5..098a53637 100644 --- a/cmd/gosqlx/cmd/lint.go +++ b/cmd/gosqlx/cmd/lint.go @@ -438,8 +438,9 @@ func runSecurityScan(sql string, filename string, outWriter io.Writer) int { fmt.Fprintf(outWriter, " [WARN] %s: could not parse for security scan: %v\n", filename, err) return 0 } + defer ast.ReleaseAST(tree) scanner := sqlsecurity.NewScanner() - result := scanner.Scan(tree) + result := mergeSecurityFindings(scanner.Scan(tree), scanner.ScanSQL(sql)) for _, f := range result.Findings { fmt.Fprintf(outWriter, " [%s] %s:%d:%d - %s (%s)\n", f.Severity, filename, f.Line, f.Column, f.Description, f.Pattern) diff --git a/cmd/gosqlx/cmd/sql_analyzer.go b/cmd/gosqlx/cmd/sql_analyzer.go index d8926f025..b3f24ff08 100644 --- a/cmd/gosqlx/cmd/sql_analyzer.go +++ b/cmd/gosqlx/cmd/sql_analyzer.go @@ -73,7 +73,7 @@ func (a *SQLAnalyzer) Analyze(astObj *ast.AST, sqlArgs ...string) (*AnalysisRepo // Run security scanner for injection patterns (tautologies, UNION-based, time-based, etc.) securityScanner := security.NewScanner() - securityResult := securityScanner.Scan(astObj) + securityResult := mergeSecurityFindings(securityScanner.Scan(astObj), securityScanner.ScanSQL(sql)) a.convertSecurityFindings(securityResult) // Calculate final metrics and scores @@ -591,3 +591,37 @@ func (a *SQLAnalyzer) convertSecurityFindings(result *security.ScanResult) { a.adjustSecurityScore(sev) } } + +// mergeSecurityFindings combines AST and raw SQL scans, preserving distinct +// descriptions while avoiding duplicate reports from the two scan paths. +func mergeSecurityFindings(results ...*security.ScanResult) *security.ScanResult { + merged := &security.ScanResult{} + type findingKey struct { + pattern security.PatternType + severity security.Severity + description string + } + seen := make(map[findingKey]bool) + for _, result := range results { + for _, finding := range result.Findings { + key := findingKey{finding.Pattern, finding.Severity, finding.Description} + if seen[key] { + continue + } + seen[key] = true + merged.Findings = append(merged.Findings, finding) + merged.TotalCount++ + switch finding.Severity { + case security.SeverityCritical: + merged.CriticalCount++ + case security.SeverityHigh: + merged.HighCount++ + case security.SeverityMedium: + merged.MediumCount++ + case security.SeverityLow: + merged.LowCount++ + } + } + } + return merged +} diff --git a/cmd/gosqlx/internal/cmdutil/input.go b/cmd/gosqlx/internal/cmdutil/input.go index 587e49962..979b36f7e 100644 --- a/cmd/gosqlx/internal/cmdutil/input.go +++ b/cmd/gosqlx/internal/cmdutil/input.go @@ -83,7 +83,7 @@ func DetectAndReadInput(input string) (*InputResult, error) { }, nil } - if strings.Contains(input, string(filepath.Separator)) || strings.HasSuffix(strings.ToLower(input), ".sql") { + if !LooksLikeSQL(input) && (strings.Contains(input, string(filepath.Separator)) || strings.HasSuffix(strings.ToLower(input), ".sql")) { return nil, fmt.Errorf("invalid file path: %w", statErr) } diff --git a/cmd/gosqlx/internal/cmdutil/input_test.go b/cmd/gosqlx/internal/cmdutil/input_test.go index 18e454524..90fd2091b 100644 --- a/cmd/gosqlx/internal/cmdutil/input_test.go +++ b/cmd/gosqlx/internal/cmdutil/input_test.go @@ -83,3 +83,30 @@ func TestLooksLikeSQL(t *testing.T) { } } } + +func TestDetectAndReadInputSQLWithSlashes(t *testing.T) { + for _, sql := range []string{"SELECT 1 UNION/**/SELECT 2", "SELECT 4 / 2", "SELECT '/tmp/query.sql'"} { + result, err := DetectAndReadInput(sql) + if err != nil { + t.Fatalf("DetectAndReadInput(%q): %v", sql, err) + } + if result.Type != InputTypeSQL || string(result.Content) != sql { + t.Fatalf("unexpected input: %+v", result) + } + } + // An existing file remains a file even when its name looks like SQL. + path := filepath.Join(t.TempDir(), "SELECT query.sql") + if err := os.WriteFile(path, []byte("SELECT 7"), 0600); err != nil { + t.Fatal(err) + } + result, err := DetectAndReadInput(path) + if err != nil { + t.Fatal(err) + } + if result.Type != InputTypeFile || string(result.Content) != "SELECT 7" { + t.Fatalf("unexpected file input: %+v", result) + } + if _, err := DetectAndReadInput(filepath.Join(t.TempDir(), "missing.sql")); err == nil { + t.Fatal("missing file should fail") + } +} diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index 7e4651b70..e1b34a661 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -326,7 +326,7 @@ func renderSelect(s *ast.SelectStatement, opts ast.FormatOptions) string { // MySQL select modifiers and TOP, between SELECT [DISTINCT] and columns. writeSelectPrologue(sb, s, f) - sb.WriteString(f.exprListSQL(s.Columns)) + sb.WriteString(selectColumnsForDialect(s.Columns, f)) if into := selectExtras(s).Into; into != nil && !into.Trailing { sb.WriteString(" ") @@ -772,6 +772,12 @@ func renderSetOperationBody(s *ast.SetOperation, opts ast.FormatOptions) string if t := s.Tail; t != nil { // ORDER BY / LIMIT / OFFSET / FETCH of the whole operation. view := t.Select() + // A set tail cannot use TOP, and SQL Server OFFSET/FETCH needs + // ORDER BY. Keep unsupported LIMIT visible for direct formatting; + // Transpile rejects this case before rendering. + if opts.Dialect != "sqlserver" || len(view.OrderBy) > 0 { + normalizeSelectForDialect(view, opts.Dialect) + } if len(view.OrderBy) > 0 { sb.WriteString(f.clauseSep()) sb.WriteString(f.kw("ORDER BY")) @@ -1249,7 +1255,7 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { } if t.Subquery != nil { sb.WriteString("(") - sb.WriteString(f.tableSubquerySQL(t.Subquery)) + sb.WriteString(tableSubqueryForDialect(t.Subquery, f)) sb.WriteString(")") } else if rows := ast.RowsFromSQL(t, f.kw); rows != "" { sb.WriteString(rows) @@ -1261,7 +1267,7 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { } } else if t.SetSubquery != nil { sb.WriteString("(") - sb.WriteString(f.tableSubquerySQL(t.SetSubquery)) + sb.WriteString(tableSubqueryForDialect(t.SetSubquery, f)) sb.WriteString(")") } else if t.DialectModifiers().Values != nil { sb.WriteString("(") diff --git a/pkg/formatter/render_select_ext.go b/pkg/formatter/render_select_ext.go index 8beb28d2e..7ba5f39a8 100644 --- a/pkg/formatter/render_select_ext.go +++ b/pkg/formatter/render_select_ext.go @@ -21,6 +21,7 @@ package formatter import ( + "reflect" "strconv" "strings" @@ -476,3 +477,90 @@ func normalizeLimitForDialect(s *ast.SelectStatement, dialect string) { clearLimit() } } + +// tableSubqueryForDialect uses the same renderer as a top-level statement so +// derived tables and JOIN operands receive target-dialect row-limit syntax. +func tableSubqueryForDialect(s ast.Statement, f *nodeFormatter) string { + if f.opts.Dialect == "" { + return f.tableSubquerySQL(s) + } + return dialectSubquerySQL(s, f.opts) +} + +// dialectSubquerySQL takes options by value so callers' closures need not +// capture (and heap-allocate) the node formatter. +func dialectSubquerySQL(s ast.Statement, parent ast.FormatOptions) string { + opts := nestedOpts(parent) + opts.NewlinePerClause = false + return formatStatement(s, opts) +} + +// selectColumnsForDialect preserves expression SQL's parentheses and aliases +// while routing embedded scalar queries through target-dialect rendering. +// Only expressions containing a query need copying; the caller's AST stays intact. +func selectColumnsForDialect(columns []ast.Expression, f *nodeFormatter) string { + plain := f.exprListSQL(columns) + // A scalar subquery always renders a SELECT keyword; without one the + // plain rendering is already correct and costs nothing extra. + if f.opts.Dialect == "" || !containsFoldASCII(plain, "select") { + return plain + } + hasQuery := false + visit := func(n ast.Node) bool { + if _, ok := n.(*ast.SubqueryExpression); ok { + hasQuery = true + } + return !hasQuery + } + queries := make([]bool, len(columns)) + for i, column := range columns { + hasQuery = false + ast.Inspect(column, visit) + queries[i] = hasQuery + } + rendered := make([]string, len(columns)) + for i, column := range columns { + if !queries[i] { + rendered[i] = f.exprSQL(column) + continue + } + value := copyRenderValue(reflect.ValueOf(column), make(map[uintptr]reflect.Value)) + copy, ok := value.Interface().(ast.Expression) + if !ok { + rendered[i] = f.exprSQL(column) + continue + } + opts := f.opts + ast.Inspect(copy, func(n ast.Node) bool { + query, ok := n.(*ast.SubqueryExpression) + if !ok { + return true + } + query.Subquery = &ast.UtilityStatement{Text: dialectSubquerySQL(query.Subquery, opts)} + return false + }) + rendered[i] = f.exprSQL(copy) + } + return strings.Join(rendered, ", ") +} + +// containsFoldASCII reports whether s contains the lower-case ASCII word sub, +// ignoring ASCII case, without allocating. +func containsFoldASCII(s, sub string) bool { + for i := 0; i+len(sub) <= len(s); i++ { + j := 0 + for ; j < len(sub); j++ { + c := s[i+j] + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + if c != sub[j] { + break + } + } + if j == len(sub) { + return true + } + } + return false +} diff --git a/pkg/formatter/wpq_nested_test.go b/pkg/formatter/wpq_nested_test.go new file mode 100644 index 000000000..fc14f9bbf --- /dev/null +++ b/pkg/formatter/wpq_nested_test.go @@ -0,0 +1,102 @@ +// 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 formatter_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" + "github.com/ajitpratap0/GoSQLX/pkg/sql/parser" +) + +func TestNestedRowLimitRenderingPreservesSourceAndCase(t *testing.T) { + const sql = `SELECT 1 + (SELECT "Mixed" FROM t ORDER BY "Mixed" LIMIT 3) AS "Alias" FROM (SELECT a FROM t LIMIT 2) q` + tree, err := parser.ParseWithDialect(sql, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + before := formatter.FormatAST(tree, ast.CompactStyle()) + for _, lower := range []bool{false, true} { + opts := ast.CompactStyle() + opts.Dialect = "oracle" + want := "FETCH FIRST 3 ROWS ONLY" + if lower { + opts.KeywordCase = ast.KeywordLower + want = strings.ToLower(want) + } + got := formatter.FormatAST(tree, opts) + if !strings.Contains(got, want) || !strings.Contains(got, `"Mixed"`) || !strings.Contains(got, `"Alias"`) { + t.Fatalf("case or quoted alias lost: %s", got) + } + reparsed, err := parser.ParseWithDialect(got, keywords.DialectOracle) + if err != nil { + t.Fatalf("target SQL %q does not parse: %v", got, err) + } + ast.ReleaseAST(reparsed) + if after := formatter.FormatAST(tree, ast.CompactStyle()); after != before { + t.Fatalf("source AST changed: before %q; after %q", before, after) + } + } +} + +func TestNestedSetTailRenderingPreservesSource(t *testing.T) { + const sql = "SELECT * FROM (SELECT a FROM t UNION (SELECT b FROM u LIMIT 1) ORDER BY 1 LIMIT 5 OFFSET 2) x" + tree, err := parser.ParseWithDialect(sql, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + before := formatter.FormatAST(tree, ast.CompactStyle()) + for _, target := range []keywords.SQLDialect{keywords.DialectOracle, keywords.DialectSQLServer} { + opts := ast.CompactStyle() + opts.Dialect = string(target) + got := formatter.FormatAST(tree, opts) + if strings.Contains(got, "LIMIT") || !strings.Contains(got, "FETCH") { + t.Fatalf("set tail not normalized: %s", got) + } + reparsed, err := parser.ParseWithDialect(got, target) + if err != nil { + t.Fatalf("target SQL %q does not parse: %v", got, err) + } + ast.ReleaseAST(reparsed) + if after := formatter.FormatAST(tree, ast.CompactStyle()); after != before { + t.Fatalf("source set tail changed: before %q; after %q", before, after) + } + } +} + +func TestSQLServerSetTailWithoutOrderDoesNotDropLimit(t *testing.T) { + tree, err := parser.ParseWithDialect("SELECT a FROM t UNION (SELECT b FROM u LIMIT 1) LIMIT 5", keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + set, ok := tree.Statements[0].(*ast.SetOperation) + if !ok || set.Tail == nil { + t.Fatal("regression requires an explicit set-operation tail") + } + opts := ast.CompactStyle() + opts.Dialect = string(keywords.DialectSQLServer) + got := formatter.FormatAST(tree, opts) + // The formatter has no error surface for this unsupported set tail. Keep + // its LIMIT visible; only Transpile may reject it rather than erase it. + if !strings.HasSuffix(got, "LIMIT 5") { + t.Fatalf("unsupported set limit silently dropped: %s", got) + } +} diff --git a/pkg/lsp/handler.go b/pkg/lsp/handler.go index f768677b3..592a4e9de 100644 --- a/pkg/lsp/handler.go +++ b/pkg/lsp/handler.go @@ -161,6 +161,8 @@ func NewHandler(server *Server) *Handler { } } +var errMethodNotFound = stderrors.New("method not found") + // HandleRequest processes an LSP request and returns a result func (h *Handler) HandleRequest(method string, params json.RawMessage) (interface{}, error) { switch method { @@ -187,7 +189,7 @@ func (h *Handler) HandleRequest(method string, params json.RawMessage) (interfac } return h.handleSemanticTokensFull(semParams) default: - return nil, fmt.Errorf("method not found: %s", method) + return nil, fmt.Errorf("%w: %s", errMethodNotFound, method) } } diff --git a/pkg/lsp/method_not_found_test.go b/pkg/lsp/method_not_found_test.go new file mode 100644 index 000000000..ebeab956c --- /dev/null +++ b/pkg/lsp/method_not_found_test.go @@ -0,0 +1,53 @@ +// 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 lsp + +import ( + "bytes" + "encoding/json" + "io" + "log" + "strings" + "testing" +) + +func TestUnknownMethodDispatch(t *testing.T) { + for _, tt := range []struct { + name string + message string + wantCode int + }{ + {"request", `{"jsonrpc":"2.0","id":7,"method":"textDocument/definition","params":{}}`, MethodNotFound}, + {"notification", `{"jsonrpc":"2.0","method":"textDocument/definition","params":{}}`, 0}, + {"handler error", `{"jsonrpc":"2.0","id":7,"method":"initialize","params":true}`, InternalError}, + } { + t.Run(tt.name, func(t *testing.T) { + var output bytes.Buffer + server := NewServer(strings.NewReader(""), &output, log.New(io.Discard, "", 0)) + server.handleMessage(json.RawMessage(tt.message)) + if tt.wantCode == 0 { + if output.Len() != 0 { + t.Fatalf("notification produced a response: %s", output.String()) + } + return + } + responses := robustnessResponses(t, &output) + responseError, ok := responses[7]["error"].(map[string]any) + if !ok || responseError["code"] != float64(tt.wantCode) { + t.Fatalf("response = %v, want error code %d", responses, tt.wantCode) + } + }) + } +} diff --git a/pkg/lsp/server.go b/pkg/lsp/server.go index 7d6567cd4..f5ea53e0a 100644 --- a/pkg/lsp/server.go +++ b/pkg/lsp/server.go @@ -424,7 +424,11 @@ func (s *Server) dispatchMessage(msg json.RawMessage) { // It's a request expecting a response result, err := s.handler.HandleRequest(req.Method, req.Params) if err != nil { - s.sendError(req.ID, InternalError, err.Error()) + code := InternalError + if errors.Is(err, errMethodNotFound) { + code = MethodNotFound + } + s.sendError(req.ID, code, err.Error()) } else { s.sendResult(req.ID, result) } diff --git a/pkg/sql/ast/query_tail.go b/pkg/sql/ast/query_tail.go index d7d764685..89462d937 100644 --- a/pkg/sql/ast/query_tail.go +++ b/pkg/sql/ast/query_tail.go @@ -26,14 +26,15 @@ import "bytes" // SelectStatement as the parser does for A UNION B ORDER BY x (where the // clause is stored on the last SELECT). type QueryTail struct { - OrderBy []OrderByExpression - Limit *int - LimitExpr Expression - LimitAll bool - Offset *int - OffsetExpr Expression - OffsetRows bool - Fetch *FetchClause + OrderBy []OrderByExpression + Limit *int + LimitExpr Expression + LimitAll bool + LimitWithTies bool + Offset *int + OffsetExpr Expression + OffsetRows bool + Fetch *FetchClause } // Select returns a SelectStatement that holds only the clauses of t, so the @@ -43,8 +44,8 @@ func (t *QueryTail) Select() *SelectStatement { return nil } s := &SelectStatement{OrderBy: t.OrderBy, Limit: t.Limit, Offset: t.Offset, Fetch: t.Fetch} - if t.LimitExpr != nil || t.LimitAll || t.OffsetExpr != nil || t.OffsetRows { - s.Extras = &SelectExtras{LimitExpr: t.LimitExpr, LimitAll: t.LimitAll, OffsetExpr: t.OffsetExpr, OffsetRows: t.OffsetRows} + if t.LimitExpr != nil || t.LimitAll || t.LimitWithTies || t.OffsetExpr != nil || t.OffsetRows { + s.Extras = &SelectExtras{LimitExpr: t.LimitExpr, LimitAll: t.LimitAll, LimitWithTies: t.LimitWithTies, OffsetExpr: t.OffsetExpr, OffsetRows: t.OffsetRows} } return s } @@ -53,14 +54,15 @@ func (t *QueryTail) Select() *SelectStatement { // from s: the parser reads them into a scratch SelectStatement first. func QueryTailFrom(s *SelectStatement) *QueryTail { return &QueryTail{ - OrderBy: s.OrderBy, - Limit: s.Limit, - LimitExpr: s.DialectExtras().LimitExpr, - LimitAll: s.DialectExtras().LimitAll, - Offset: s.Offset, - OffsetExpr: s.DialectExtras().OffsetExpr, - OffsetRows: s.DialectExtras().OffsetRows, - Fetch: s.Fetch, + OrderBy: s.OrderBy, + Limit: s.Limit, + LimitExpr: s.DialectExtras().LimitExpr, + LimitAll: s.DialectExtras().LimitAll, + LimitWithTies: s.DialectExtras().LimitWithTies, + Offset: s.Offset, + OffsetExpr: s.DialectExtras().OffsetExpr, + OffsetRows: s.DialectExtras().OffsetRows, + Fetch: s.Fetch, } } @@ -114,4 +116,7 @@ func (t *QueryTail) release(mode releaseMode) { } releaseExpression(t.LimitExpr, mode) releaseExpression(t.OffsetExpr, mode) + if t.Fetch != nil { + releaseExpression(t.Fetch.FetchExpr, mode) + } } diff --git a/pkg/sql/ast/query_tail_test.go b/pkg/sql/ast/query_tail_test.go new file mode 100644 index 000000000..3b826c2b8 --- /dev/null +++ b/pkg/sql/ast/query_tail_test.go @@ -0,0 +1,46 @@ +// 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 ast + +import "testing" + +func TestQueryTailPreservesTies(t *testing.T) { + limit := 5 + for _, fetch := range []bool{false, true} { + s := &SelectStatement{Limit: &limit, Extras: &SelectExtras{LimitWithTies: true}} + if fetch { + s.Limit = nil + s.Extras = nil + s.Fetch = &FetchClause{WithTies: true} + } + tail := QueryTailFrom(s) + view := tail.Select() + if fetch { + if view.Fetch == nil || !view.Fetch.WithTies { + t.Fatal("FETCH ties lost") + } + } else if !view.DialectExtras().LimitWithTies { + t.Fatal("LIMIT ties lost") + } + again := QueryTailFrom(view).Select() + if fetch { + if again.Fetch == nil || !again.Fetch.WithTies { + t.Fatal("FETCH ties lost after round trip") + } + } else if !again.DialectExtras().LimitWithTies { + t.Fatal("LIMIT ties lost after round trip") + } + } +} diff --git a/pkg/transpiler/row_limits.go b/pkg/transpiler/row_limits.go index 67111cc77..a452f61dc 100644 --- a/pkg/transpiler/row_limits.go +++ b/pkg/transpiler/row_limits.go @@ -41,6 +41,13 @@ func rowLimitRule(to keywords.SQLDialect) RewriteRule { rewriteErr = fmt.Errorf("DML TOP cannot be transpiled to %s", to) return false } + if tail, ok := n.(*ast.QueryTail); ok { + view := tail.Select() + if rewriteErr = normalizeTies(view, to); rewriteErr != nil { + return false + } + *tail = *ast.QueryTailFrom(view) + } if set, ok := n.(*ast.SetOperation); ok && to == keywords.DialectSQLServer { if right, ok := set.Right.(*ast.SelectStatement); ok && !set.RightParenthesized { if setLevel == nil { @@ -50,6 +57,9 @@ func rowLimitRule(to keywords.SQLDialect) RewriteRule { } if set.Tail != nil { view := set.Tail.Select() + if rewriteErr = normalizeTies(view, to); rewriteErr != nil { + return false + } if rewriteErr = sqlServerSetLimit(view); rewriteErr != nil { return false } @@ -95,8 +105,7 @@ func rowLimitRule(to keywords.SQLDialect) RewriteRule { if !ok { return true } - if to == keywords.DialectSQLServer && s.DialectExtras().LimitWithTies { - rewriteErr = fmt.Errorf("LIMIT WITH TIES cannot be transpiled to SQL Server") + if rewriteErr = normalizeTies(s, to); rewriteErr != nil { return false } if s.Top != nil && to != keywords.DialectSQLServer { @@ -132,12 +141,21 @@ func rowLimitRule(to keywords.SQLDialect) RewriteRule { rewriteErr = sqlServerSetLimit(s) return rewriteErr == nil } - // With an offset, SQL Server requires OFFSET/FETCH; leave that case - // to the formatter. ORDER BY alone does not prevent use of TOP. + // With an offset, SQL Server requires OFFSET/FETCH and ORDER BY. + // 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 } + if to == keywords.DialectSQLServer && (s.HasOffset() || s.Fetch != nil) { + if len(s.OrderBy) == 0 { + unspecifiedOrder := &ast.SubqueryExpression{Subquery: &ast.SelectStatement{ + Columns: []ast.Expression{&ast.LiteralValue{Type: "NULL"}}, + }} + s.OrderBy = []ast.OrderByExpression{{Ascending: true, Expression: unspecifiedOrder}} + } + sqlServerOffset(s) + } return true }) return rewriteErr @@ -147,7 +165,7 @@ func rowLimitRule(to keywords.SQLDialect) RewriteRule { // sqlServerSetLimit keeps a set operation's row limit in its trailing clauses. // Both unparenthesized right SELECTs and explicit QueryTail views use this path. func sqlServerSetLimit(s *ast.SelectStatement) error { - if !s.HasLimit() && !s.HasOffset() { + if !s.HasLimit() && !s.HasOffset() && s.Fetch == nil { return nil } if len(s.OrderBy) == 0 { @@ -172,8 +190,50 @@ func sqlServerSetLimit(s *ast.SelectStatement) error { s.Extras.LimitExpr = nil s.Extras.LimitAll = false } + sqlServerOffset(s) + return nil +} + +// sqlServerOffset supplies the mandatory offset before FETCH and spells bare +// OFFSET using SQL Server's required ROWS suffix. +func sqlServerOffset(s *ast.SelectStatement) { + if s.Fetch != nil && s.Fetch.OffsetValue == nil && !s.HasOffset() { + zero := 0 + s.Offset = &zero + } if s.HasOffset() { s.EnsureExtras().OffsetRows = true } - return nil +} + +// normalizeTies preserves tied-row semantics, refusing targets without an +// equivalent FETCH WITH TIES clause rather than silently lowering to ONLY. +func normalizeTies(s *ast.SelectStatement, to keywords.SQLDialect) error { + limitTies := s.DialectExtras().LimitWithTies + fetchTies := s.Fetch != nil && s.Fetch.WithTies + if !limitTies && !fetchTies { + return nil + } + if limitTies && to == keywords.DialectClickHouse { + return nil + } + switch to { + case keywords.DialectPostgreSQL, keywords.DialectOracle, keywords.DialectGeneric, "": + if limitTies { + fetch := &ast.FetchClause{FetchType: "FIRST", WithTies: true} + if s.Limit != nil { + value := int64(*s.Limit) + fetch.FetchValue = &value + } else { + fetch.FetchExpr = s.DialectExtras().LimitExpr + } + s.Fetch = fetch + s.Limit = nil + s.EnsureExtras().LimitExpr = nil + s.Extras.LimitWithTies = false + } + return nil + default: + return fmt.Errorf("WITH TIES cannot be transpiled to %s", to) + } } diff --git a/pkg/transpiler/row_limits_test.go b/pkg/transpiler/row_limits_test.go index 598ae7184..ccc61c24d 100644 --- a/pkg/transpiler/row_limits_test.go +++ b/pkg/transpiler/row_limits_test.go @@ -247,3 +247,78 @@ func TestTranspileTopSetOperandConflictingTailOrder(t *testing.T) { t.Fatalf("got %v; want conflicting ORDER BY error", err) } } + +func TestTranspileSQLServerOffsetFetch(t *testing.T) { + cases := []struct{ sql, want string }{ + {"SELECT a FROM t LIMIT 5 OFFSET 2", "SELECT a FROM t ORDER BY (SELECT NULL) OFFSET 2 ROWS FETCH NEXT 5 ROWS ONLY"}, + {"SELECT a FROM t ORDER BY a LIMIT 5 OFFSET 2", "SELECT a FROM t ORDER BY a OFFSET 2 ROWS FETCH NEXT 5 ROWS ONLY"}, + {"SELECT a FROM t LIMIT 5", "SELECT TOP 5 a FROM t"}, + {"SELECT a FROM t ORDER BY 1 FETCH FIRST 5 ROWS ONLY", "SELECT a FROM t ORDER BY 1 OFFSET 0 ROWS FETCH FIRST 5 ROWS ONLY"}, + {"SELECT a FROM t FETCH FIRST 5 ROWS ONLY", "SELECT a FROM t ORDER BY (SELECT NULL) OFFSET 0 ROWS FETCH FIRST 5 ROWS ONLY"}, + {"SELECT a FROM t OFFSET 3", "SELECT a FROM t ORDER BY (SELECT NULL) OFFSET 3 ROWS"}, + {"SELECT a FROM t UNION SELECT b FROM u ORDER BY 1 FETCH FIRST 5 ROWS ONLY", "SELECT a FROM t UNION SELECT b FROM u ORDER BY 1 OFFSET 0 ROWS FETCH FIRST 5 ROWS ONLY"}, + } + for _, source := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectSQLite} { + for _, tc := range cases { + t.Run(string(source)+"/"+tc.sql, func(t *testing.T) { + got, err := transpiler.Transpile(tc.sql, source, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + tree, err := parser.ParseWithDialect(got, keywords.DialectSQLServer) + if err != nil { + t.Fatalf("target SQL does not parse: %v", err) + } + defer ast.ReleaseAST(tree) + }) + } + } + for _, sql := range []string{"SELECT a FROM t UNION SELECT b FROM u FETCH FIRST 5 ROWS ONLY", "SELECT a FROM t UNION (SELECT b FROM u) FETCH FIRST 5 ROWS ONLY"} { + if got, err := transpiler.Transpile(sql, keywords.DialectPostgreSQL, keywords.DialectSQLServer); err == nil || got != "" { + t.Fatalf("got %q, %v; want set operation ORDER BY refusal", got, err) + } + } +} + +func TestTranspileWithTiesNeverLost(t *testing.T) { + for _, sql := range []string{ + "SELECT a FROM t ORDER BY 1 LIMIT 5 WITH TIES", + "SELECT a FROM t ORDER BY 1 LIMIT 5 WITH TIES OFFSET 2", + "SELECT a FROM t UNION ALL SELECT b FROM u ORDER BY 1 LIMIT 5 WITH TIES", + "SELECT a FROM t UNION ALL (SELECT b FROM u) ORDER BY 1 LIMIT 5 WITH TIES", + "SELECT a FROM t UNION ALL (SELECT b FROM u ORDER BY b LIMIT 1) ORDER BY 1 LIMIT 5 WITH TIES", + "SELECT a FROM t UNION ALL (SELECT b FROM u ORDER BY b LIMIT 1) ORDER BY 1 LIMIT 5 WITH TIES OFFSET 2", + "SELECT a FROM t ORDER BY 1 FETCH FIRST 5 ROWS WITH TIES", + "SELECT a FROM t UNION ALL (SELECT b FROM u) ORDER BY 1 FETCH FIRST 5 ROWS WITH TIES", + "SELECT a FROM t UNION ALL (SELECT b FROM u ORDER BY b LIMIT 1) ORDER BY 1 FETCH FIRST 5 ROWS WITH TIES", + } { + for _, target := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectOracle, keywords.DialectMySQL, keywords.DialectSQLite, keywords.DialectSQLServer} { + t.Run(string(target)+"/"+sql, func(t *testing.T) { + got, err := transpiler.Transpile(sql, keywords.DialectClickHouse, target) + if err != nil { + if target == keywords.DialectPostgreSQL || target == keywords.DialectOracle { + t.Fatalf("target supports FETCH WITH TIES: %v", err) + } + if got != "" { + t.Fatalf("output on error: %q", got) + } + return + } + if target != keywords.DialectPostgreSQL && target != keywords.DialectOracle { + t.Fatalf("target cannot express FETCH WITH TIES: got %q", got) + } + if !strings.Contains(got, "WITH TIES") { + t.Fatalf("ties lost: %s", got) + } + tree, err := parser.ParseWithDialect(got, target) + if err != nil { + t.Fatalf("target SQL does not parse: %v", err) + } + defer ast.ReleaseAST(tree) + }) + } + } +} diff --git a/pkg/transpiler/wpq_nested_test.go b/pkg/transpiler/wpq_nested_test.go new file mode 100644 index 000000000..b7024b21f --- /dev/null +++ b/pkg/transpiler/wpq_nested_test.go @@ -0,0 +1,60 @@ +// 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 TestTranspileNestedRowLimitsTargetDialect(t *testing.T) { + tests := []struct { + name, sql string + from, to keywords.SQLDialect + want string + }{ + {"derived oracle", "SELECT * FROM (SELECT a FROM t ORDER BY a LIMIT 3) s", keywords.DialectPostgreSQL, keywords.DialectOracle, "FETCH FIRST 3 ROWS ONLY"}, + {"join oracle", "SELECT * FROM t JOIN (SELECT a FROM u ORDER BY a LIMIT 3) s ON t.a = s.a", keywords.DialectPostgreSQL, keywords.DialectOracle, "FETCH FIRST 3 ROWS ONLY"}, + {"scalar oracle", "SELECT (SELECT a FROM t ORDER BY a LIMIT 3) AS a", keywords.DialectPostgreSQL, keywords.DialectOracle, "FETCH FIRST 3 ROWS ONLY"}, + {"scalar expression oracle", "SELECT 1 + (SELECT a FROM t ORDER BY a LIMIT 3) AS a", keywords.DialectPostgreSQL, keywords.DialectOracle, "FETCH FIRST 3 ROWS ONLY"}, + {"nested top oracle", "SELECT * FROM (SELECT TOP 3 a FROM t) s", keywords.DialectSQLServer, keywords.DialectOracle, "FETCH FIRST 3 ROWS ONLY"}, + {"derived set oracle", "SELECT * FROM (SELECT a FROM t UNION SELECT b FROM u ORDER BY 1 LIMIT 5) x", keywords.DialectPostgreSQL, keywords.DialectOracle, "FETCH FIRST 5 ROWS ONLY"}, + {"derived sqlserver", "SELECT * FROM (SELECT a FROM t LIMIT 3) s", keywords.DialectPostgreSQL, keywords.DialectSQLServer, "SELECT TOP 3 a FROM t"}, + {"join sqlserver", "SELECT * FROM t JOIN (SELECT a FROM u LIMIT 3) s ON t.a = s.a", keywords.DialectPostgreSQL, keywords.DialectSQLServer, "SELECT TOP 3 a FROM u"}, + {"scalar sqlserver", "SELECT (SELECT a FROM t LIMIT 3) AS a", keywords.DialectPostgreSQL, keywords.DialectSQLServer, "SELECT TOP 3 a FROM t"}, + {"derived offset sqlserver", "SELECT * FROM (SELECT a FROM t ORDER BY a LIMIT 3 OFFSET 1) s", keywords.DialectPostgreSQL, keywords.DialectSQLServer, "ORDER BY a OFFSET 1 ROWS FETCH NEXT 3 ROWS ONLY"}, + } + 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 !strings.Contains(got, tt.want) || strings.Contains(got, "LIMIT") { + t.Fatalf("target row limit missing: got %q; want %q", got, tt.want) + } + tree, err := parser.ParseWithDialect(got, tt.to) + if err != nil { + t.Fatalf("target SQL %q does not parse: %v", got, err) + } + defer ast.ReleaseAST(tree) + }) + } +}