diff --git a/cmd/gosqlx/cmd/insert_update_forms_format_test.go b/cmd/gosqlx/cmd/insert_update_forms_format_test.go new file mode 100644 index 00000000..ef872075 --- /dev/null +++ b/cmd/gosqlx/cmd/insert_update_forms_format_test.go @@ -0,0 +1,99 @@ +// 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" + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +// TestFormatCommand_InsertUpdateForms is a regression test for the #610 +// grammar additions: the `gosqlx format` renderer dropped UPDATE ... FROM +// sources and rendered INSERT ... DEFAULT VALUES as a bare INSERT INTO t, so +// `format -i` rewrote files with SQL of different meaning or invalid SQL. +func TestFormatCommand_InsertUpdateForms(t *testing.T) { + cases := []struct { + name, dialect, in, want string + }{ + {"update from", "postgresql", + "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + "UPDATE t SET a = s.b FROM s WHERE t.id = s.id"}, + {"update from list", "postgresql", + "UPDATE t SET a = s.b FROM s, u AS v WHERE t.id = s.id", + "UPDATE t SET a = s.b FROM s, u AS v WHERE t.id = s.id"}, + {"update from", "sqlserver", + "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + "UPDATE t SET a = s.b FROM s WHERE t.id = s.id"}, + {"update from no where", "postgresql", + "UPDATE t SET a = s.b FROM s", + "UPDATE t SET a = s.b FROM s"}, + {"default values", "postgresql", + "INSERT INTO t DEFAULT VALUES", + "INSERT INTO t DEFAULT VALUES"}, + {"default values", "sqlserver", + "INSERT INTO t DEFAULT VALUES", + "INSERT INTO t DEFAULT VALUES"}, + {"default values", "sqlite", + "INSERT INTO t DEFAULT VALUES", + "INSERT INTO t DEFAULT VALUES"}, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name+"/"+tc.dialect, func(t *testing.T) { + var out, errBuf bytes.Buffer + f := NewFormatter(&out, &errBuf, CLIFormatterOptions{Compact: true, Uppercase: true, Dialect: tc.dialect}) + got, err := f.formatSQL(tc.in) + if err != nil { + t.Fatalf("formatSQL(%q) failed: %v", tc.in, err) + } + got = strings.TrimSpace(got) + if got != tc.want { + t.Fatalf("formatSQL mismatch\n got: %q\nwant: %q", got, tc.want) + } + + // The output must mean the same thing: re-parse it and compare + // the canonical SQL() of both trees. + d := keywords.SQLDialect(tc.dialect) + inTree, err := gosqlx.ParseWithDialect(tc.in, d) + if err != nil { + t.Fatalf("parse(%q) failed: %v", tc.in, err) + } + outTree, err := gosqlx.ParseWithDialect(got, d) + if err != nil { + t.Fatalf("re-parse of %q failed: %v", got, err) + } + if inTree.SQL() != outTree.SQL() { + t.Fatalf("formatted SQL changed meaning\n in: %q\n out: %q", inTree.SQL(), outTree.SQL()) + } + }) + } + + t.Run("readable update from", func(t *testing.T) { + var out, errBuf bytes.Buffer + f := NewFormatter(&out, &errBuf, CLIFormatterOptions{IndentSize: 2, Uppercase: true, Dialect: "postgresql"}) + got, err := f.formatSQL("UPDATE t SET a = s.b FROM s WHERE t.id = s.id") + if err != nil { + t.Fatalf("formatSQL failed: %v", err) + } + want := "UPDATE t\nSET a = s.b\nFROM s\nWHERE t.id = s.id" + if strings.TrimSpace(got) != want { + t.Fatalf("readable output mismatch\n got: %q\nwant: %q", strings.TrimSpace(got), want) + } + }) +} diff --git a/cmd/gosqlx/cmd/sql_formatter.go b/cmd/gosqlx/cmd/sql_formatter.go index c448195b..489d1b49 100644 --- a/cmd/gosqlx/cmd/sql_formatter.go +++ b/cmd/gosqlx/cmd/sql_formatter.go @@ -297,6 +297,11 @@ func (f *SQLFormatter) formatInsert(stmt *ast.InsertStatement) error { f.builder.WriteString(")") } + if stmt.DefaultValues { + f.writeNewline() + f.writeKeyword("DEFAULT VALUES") + } + if len(stmt.Values) > 0 { f.writeNewline() f.writeKeyword("VALUES") @@ -359,6 +364,14 @@ func (f *SQLFormatter) formatUpdate(stmt *ast.UpdateStatement) error { } } + // PostgreSQL/SQL Server UPDATE ... FROM source list (#610) + if len(stmt.From) > 0 { + f.writeNewline() + f.writeKeyword("FROM") + f.builder.WriteString(" ") + f.formatTableReferences(stmt.From) + } + if stmt.Where != nil { f.writeNewline() f.writeKeyword("WHERE") diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index d21f9849..08661ca2 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -396,13 +396,26 @@ func renderInsert(i *ast.InsertStatement, opts ast.FormatOptions) string { sb.WriteString(" ") sb.WriteString(i.TableName) - if len(i.Columns) > 0 { + // MySQL INSERT ... SET form: Columns[k] = Values[0][k]. + setForm := i.SetSyntax && i.Query == nil && len(i.Columns) > 0 && + len(i.Values) == 1 && len(i.Values[0]) == len(i.Columns) + + if len(i.Columns) > 0 && !setForm { sb.WriteString(" (") sb.WriteString(exprListSQL(i.Columns)) sb.WriteString(")") } - if i.Query != nil { + if setForm { + sb.WriteString(f.clauseSep()) + sb.WriteString(f.kw("SET")) + sb.WriteString(" ") + sets := make([]string, len(i.Columns)) + for k, col := range i.Columns { + sets[k] = exprSQL(col) + " = " + exprSQL(i.Values[0][k]) + } + sb.WriteString(strings.Join(sets, ", ")) + } else if i.Query != nil { sb.WriteString(f.clauseSep()) sb.WriteString(FormatStatement(i.Query, opts)) } else if len(i.Values) > 0 { @@ -418,6 +431,9 @@ func renderInsert(i *ast.InsertStatement, opts ast.FormatOptions) string { rows[idx] = "(" + strings.Join(vals, ", ") + ")" } sb.WriteString(strings.Join(rows, ", ")) + } else if i.DefaultValues { + sb.WriteString(f.clauseSep()) + sb.WriteString(f.kw("DEFAULT VALUES")) } if i.OnConflict != nil { diff --git a/pkg/formatter/render_insert_forms_test.go b/pkg/formatter/render_insert_forms_test.go new file mode 100644 index 00000000..498eb498 --- /dev/null +++ b/pkg/formatter/render_insert_forms_test.go @@ -0,0 +1,133 @@ +// 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" +) + +// TestRenderInsertForms checks that FormatAST renders the #610 INSERT and +// UPDATE forms and that its output re-parses to the same statement. +func TestRenderInsertForms(t *testing.T) { + tests := []struct { + name string + dialect keywords.SQLDialect + sql string + contains []string + excludes []string + // compactOnly skips the readable style: it renders a nested query + // source with its own ";" (#602), so RETURNING after it would not + // re-parse until that is fixed. + compactOnly bool + }{ + { + name: "default values", dialect: keywords.DialectPostgreSQL, + sql: "INSERT INTO t DEFAULT VALUES RETURNING id", + contains: []string{"DEFAULT VALUES", "RETURNING id"}, + }, + { + name: "default values sqlserver", dialect: keywords.DialectSQLServer, + sql: "INSERT INTO t DEFAULT VALUES", + contains: []string{"DEFAULT VALUES"}, + }, + { + name: "default values sqlite", dialect: keywords.DialectSQLite, + sql: "INSERT INTO t DEFAULT VALUES", + contains: []string{"DEFAULT VALUES"}, + }, + { + name: "mysql set", dialect: keywords.DialectMySQL, + sql: "INSERT INTO t SET a = 1, b = 2", + contains: []string{"SET a = 1, b = 2"}, + excludes: []string{"(a, b)", "VALUES"}, + }, + { + name: "mariadb set", dialect: keywords.DialectMariaDB, + sql: "INSERT INTO t SET a = 1, b = 'x'", + contains: []string{"SET a = 1, b = 'x'"}, + excludes: []string{"(a, b)", "VALUES"}, + }, + { + name: "returning after select source", dialect: keywords.DialectPostgreSQL, + sql: "INSERT INTO t SELECT * FROM s RETURNING id", + contains: []string{"RETURNING id"}, + compactOnly: true, + }, + { + name: "parenthesized source", dialect: keywords.DialectGeneric, + sql: "INSERT INTO t (SELECT * FROM s)", + contains: []string{"INSERT INTO t SELECT * FROM s"}, + excludes: []string{"(SELECT"}, + compactOnly: true, + }, + { + name: "update from", dialect: keywords.DialectPostgreSQL, + sql: "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + contains: []string{"FROM s"}, + }, + } + + styles := []struct { + name string + opts ast.FormatOptions + }{ + {"compact", ast.CompactStyle()}, + {"readable", ast.ReadableStyle()}, + } + + for _, tt := range tests { + for _, st := range styles { + if tt.compactOnly && st.name != "compact" { + continue + } + t.Run(tt.name+"/"+st.name, func(t *testing.T) { + tree, err := parser.ParseWithDialect(tt.sql, tt.dialect) + if err != nil { + t.Fatalf("ParseWithDialect(%q, %q): %v", tt.sql, tt.dialect, err) + } + defer ast.ReleaseAST(tree) + want := tree.SQL() + + out := formatter.FormatAST(tree, st.opts) + flat := strings.Join(strings.Fields(out), " ") + for _, c := range tt.contains { + if !strings.Contains(flat, c) { + t.Errorf("FormatAST = %q, want it to contain %q", out, c) + } + } + for _, e := range tt.excludes { + if strings.Contains(flat, e) { + t.Errorf("FormatAST = %q, must not contain %q", out, e) + } + } + + tree2, err := parser.ParseWithDialect(out, tt.dialect) + if err != nil { + t.Fatalf("re-parse of FormatAST output %q: %v", out, err) + } + defer ast.ReleaseAST(tree2) + if got := tree2.SQL(); got != want { + t.Fatalf("round-trip SQL() = %q, want %q", got, want) + } + }) + } + } +} diff --git a/pkg/schema/validator.go b/pkg/schema/validator.go index 59700953..a69cfeb8 100644 --- a/pkg/schema/validator.go +++ b/pkg/schema/validator.go @@ -285,6 +285,26 @@ func (v *Validator) validateUpdate(s *ast.UpdateStatement) []ValidationError { tableMap[tableName] = tableName } + // UPDATE ... FROM: check the extra tables exist and make their names and + // aliases resolvable, as validateSelect does for its FROM list. + for _, ref := range s.From { + if ref.Name == "" { + continue // subquery, skip + } + if _, ok := v.Schema.GetTable(ref.Name); !ok { + errors = append(errors, ValidationError{ + Message: fmt.Sprintf("table %q does not exist in schema", ref.Name), + Severity: "error", + Suggestion: v.suggestTable(ref.Name), + }) + } + if ref.Alias != "" { + tableMap[ref.Alias] = ref.Name + } else { + tableMap[ref.Name] = ref.Name + } + } + // Validate SET column names for _, upd := range s.Assignments { colName := extractColumnName(upd.Column) diff --git a/pkg/schema/validator_test.go b/pkg/schema/validator_test.go index c336504d..84ab58e0 100644 --- a/pkg/schema/validator_test.go +++ b/pkg/schema/validator_test.go @@ -613,3 +613,43 @@ func TestSchema_CaseInsensitiveColumnLookup(t *testing.T) { t.Fatal("expected case-insensitive column lookup to succeed") } } + +// TestValidator_UpdateFrom pins that UPDATE ... FROM tables and aliases are +// resolvable and checked for existence (#610 review). +func TestValidator_UpdateFrom(t *testing.T) { + v := NewValidator(helperSchema()) + + t.Run("valid", func(t *testing.T) { + for _, sql := range []string{ + "UPDATE users SET name = o.status FROM orders o WHERE users.id = o.user_id", + "UPDATE users SET name = 'x' FROM orders WHERE users.id = orders.user_id", + } { + errs, err := v.Validate(sql) + if err != nil { + t.Fatalf("Validate(%q): %v", sql, err) + } + if len(errs) != 0 { + t.Errorf("Validate(%q): unexpected errors: %v", sql, errs) + } + } + }) + + t.Run("missing table", func(t *testing.T) { + errs, err := v.Validate("UPDATE users SET name = 'x' FROM nosuch WHERE users.id = nosuch.id") + if err != nil { + t.Fatalf("Validate: %v", err) + } + found := false + for _, e := range errs { + if strings.Contains(e.Message, `table "nosuch" does not exist`) { + found = true + } + if strings.Contains(e.Message, "not referenced in FROM clause") { + t.Errorf("unexpected misleading error: %q", e.Message) + } + } + if !found { + t.Errorf("expected missing-table error, got %v", errs) + } + }) +} diff --git a/pkg/sql/ast/ast_statements.go b/pkg/sql/ast/ast_statements.go index 1e54d066..158c1499 100644 --- a/pkg/sql/ast/ast_statements.go +++ b/pkg/sql/ast/ast_statements.go @@ -256,6 +256,12 @@ type InsertStatement struct { OnConflict *OnConflict OnDuplicateKey *UpsertClause // MySQL: ON DUPLICATE KEY UPDATE Pos models.Location // Source position of the INSERT keyword (1-based line and column) + // DefaultValues marks INSERT INTO t DEFAULT VALUES (no Columns, Values or Query). + DefaultValues bool + // SetSyntax marks the MySQL/MariaDB INSERT INTO t SET col = expr, ... form. + // The assignments are stored as Columns plus a single Values row + // (Columns[i] = Values[0][i]); renderers emit SET when the shapes match. + SetSyntax bool } func (i *InsertStatement) statementNode() {} diff --git a/pkg/sql/ast/pool_statement_release.go b/pkg/sql/ast/pool_statement_release.go index f680ee28..3b826574 100644 --- a/pkg/sql/ast/pool_statement_release.go +++ b/pkg/sql/ast/pool_statement_release.go @@ -130,6 +130,8 @@ func releaseInsertStatement(stmt *InsertStatement, mode releaseMode) { stmt.TableName = "" stmt.Pos = models.Location{} + stmt.DefaultValues = false + stmt.SetSyntax = false // Return to pool if mode == releaseAll { diff --git a/pkg/sql/ast/sql.go b/pkg/sql/ast/sql.go index 026a71b7..4cde33a5 100644 --- a/pkg/sql/ast/sql.go +++ b/pkg/sql/ast/sql.go @@ -1179,6 +1179,13 @@ func (i *InsertStatement) SQL() string { return buf.String() } +// isSetForm reports whether the statement can be rendered in the MySQL +// INSERT ... SET form recorded by SetSyntax. +func (i *InsertStatement) isSetForm() bool { + return i.SetSyntax && i.Query == nil && len(i.Columns) > 0 && + len(i.Values) == 1 && len(i.Values[0]) == len(i.Columns) +} + func (i *InsertStatement) writeSQL(buf *bytes.Buffer) { if i == nil { return @@ -1192,18 +1199,32 @@ func (i *InsertStatement) writeSQL(buf *bytes.Buffer) { buf.WriteString("INSERT INTO ") buf.WriteString(i.TableName) - if len(i.Columns) > 0 { - buf.WriteString(" (") - writeExprList(buf, i.Columns) - buf.WriteByte(')') - } + if i.isSetForm() { + buf.WriteString(" SET ") + for idx, col := range i.Columns { + if idx > 0 { + buf.WriteString(", ") + } + writeExpr(buf, col) + buf.WriteString(" = ") + writeExpr(buf, i.Values[0][idx]) + } + } else { + if len(i.Columns) > 0 { + buf.WriteString(" (") + writeExprList(buf, i.Columns) + buf.WriteByte(')') + } - if i.Query != nil { - buf.WriteByte(' ') - writeStmt(buf, i.Query) - } else if len(i.Values) > 0 { - buf.WriteString(" VALUES ") - writeValueRows(buf, i.Values) + if i.Query != nil { + buf.WriteByte(' ') + writeStmt(buf, i.Query) + } else if len(i.Values) > 0 { + buf.WriteString(" VALUES ") + writeValueRows(buf, i.Values) + } else if i.DefaultValues { + buf.WriteString(" DEFAULT VALUES") + } } if i.OnConflict != nil { diff --git a/pkg/sql/ast/sql_insert_forms_test.go b/pkg/sql/ast/sql_insert_forms_test.go new file mode 100644 index 00000000..936b56a7 --- /dev/null +++ b/pkg/sql/ast/sql_insert_forms_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 ast + +import "testing" + +// TestInsertStatementSQL_Forms covers the DEFAULT VALUES and MySQL SET +// forms of InsertStatement.SQL() (#610). +func TestInsertStatementSQL_Forms(t *testing.T) { + ab := func() []Expression { return []Expression{&Identifier{Name: "a"}, &Identifier{Name: "b"}} } + one := func() Expression { return &LiteralValue{Value: 1, Type: "INTEGER"} } + x := func() Expression { return &LiteralValue{Value: "x", Type: "STRING"} } + + tests := []struct { + name string + stmt *InsertStatement + want string + }{ + { + name: "default values", + stmt: &InsertStatement{TableName: "t", DefaultValues: true}, + want: "INSERT INTO t DEFAULT VALUES", + }, + { + name: "default values returning", + stmt: &InsertStatement{TableName: "t", DefaultValues: true, Returning: []Expression{&Identifier{Name: "id"}}}, + want: "INSERT INTO t DEFAULT VALUES RETURNING id", + }, + { + name: "set form", + stmt: &InsertStatement{TableName: "t", SetSyntax: true, Columns: ab(), Values: [][]Expression{{one(), x()}}}, + want: "INSERT INTO t SET a = 1, b = 'x'", + }, + { + name: "set syntax with two rows falls back to VALUES", + stmt: &InsertStatement{TableName: "t", SetSyntax: true, Columns: ab(), Values: [][]Expression{{one(), x()}, {one(), x()}}}, + want: "INSERT INTO t (a, b) VALUES (1, 'x'), (1, 'x')", + }, + { + name: "set syntax with cell count mismatch falls back to VALUES", + stmt: &InsertStatement{TableName: "t", SetSyntax: true, Columns: ab(), Values: [][]Expression{{one()}}}, + want: "INSERT INTO t (a, b) VALUES (1)", + }, + { + name: "values win over DefaultValues", + stmt: &InsertStatement{TableName: "t", DefaultValues: true, Columns: ab(), Values: [][]Expression{{one(), x()}}}, + want: "INSERT INTO t (a, b) VALUES (1, 'x')", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.stmt.SQL(); got != tt.want { + t.Fatalf("SQL() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestInsertStatementPool_ResetsFormFlags pins that the #610 form flags do +// not leak through the InsertStatement pool. +func TestInsertStatementPool_ResetsFormFlags(t *testing.T) { + stmt := GetInsertStatement() + stmt.DefaultValues = true + stmt.SetSyntax = true + PutInsertStatement(stmt) + + // releaseInsertStatement resets fields in place before pooling. + if stmt.DefaultValues || stmt.SetSyntax { + t.Fatalf("released InsertStatement kept DefaultValues=%v SetSyntax=%v", stmt.DefaultValues, stmt.SetSyntax) + } +} diff --git a/pkg/sql/parser/cte.go b/pkg/sql/parser/cte.go index eee84623..d67ef851 100644 --- a/pkg/sql/parser/cte.go +++ b/pkg/sql/parser/cte.go @@ -227,6 +227,12 @@ func (p *Parser) parseCommonTableExpr() (*ast.CommonTableExpr, error) { } p.advance() // Consume ( + // A CTE body is a nested scope: the INSERT-source RETURNING stop does not + // apply inside it (#610). + prevReturning := p.dmlReturning + p.dmlReturning = false + defer func() { p.dmlReturning = prevReturning }() + // Parse the inner statement - handle SELECT with set operations support var stmt ast.Statement var err error diff --git a/pkg/sql/parser/dml_insert.go b/pkg/sql/parser/dml_insert.go index 7c54df12..a91bc42e 100644 --- a/pkg/sql/parser/dml_insert.go +++ b/pkg/sql/parser/dml_insert.go @@ -45,7 +45,7 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { // Parse column list if present columns := make([]ast.Expression, 0) - if p.isType(models.TokenTypeLParen) { + if p.isType(models.TokenTypeLParen) && !p.isInsertQueryParenStart() { p.advance() // Consume ( for { @@ -85,6 +85,7 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { // Parse VALUES or SELECT var values [][]ast.Expression var query ast.QueryExpression + var defaultValues, setSyntax bool // ClickHouse-only: INSERT INTO t [(cols)] FORMAT with the data // payload being external (network / file). Short-circuit here — there is @@ -102,12 +103,10 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { } switch { - case p.isType(models.TokenTypeSelect), p.isType(models.TokenTypeWith): - // INSERT ... SELECT, or INSERT ... WITH cte AS (...) SELECT (#537). - // parseSubquery consumes SELECT itself, or delegates WITH to - // parseWithStatement, which attaches the WITH clause to the query - // (SelectStatement.With, or SetOperation.Left.With). - stmt, err := p.parseSubquery() + case p.isType(models.TokenTypeSelect), p.isType(models.TokenTypeWith), p.isInsertQueryParenStart(): + // INSERT ... SELECT, INSERT ... WITH cte AS (...) SELECT (#537), or a + // parenthesized query INSERT INTO t [(cols)] (SELECT ...) (#610). + stmt, err := p.parseInsertSourceQuery() if err != nil { return nil, err } @@ -169,6 +168,48 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { } p.advance() // Consume comma between rows } + case p.isType(models.TokenTypeDefault) && p.peekToken().Token.Type == models.TokenTypeValues: + // INSERT INTO t DEFAULT VALUES (PostgreSQL, SQL Server, SQLite). + if len(columns) > 0 { + return nil, goerrors.InvalidSyntaxError( + "DEFAULT VALUES cannot follow a column list", + p.currentLocation(), + "", + ) + } + p.advance() // Consume DEFAULT + p.advance() // Consume VALUES + defaultValues = true + case p.isType(models.TokenTypeSet) && (p.dialect == string(keywords.DialectMySQL) || p.isMariaDB()): + // MySQL/MariaDB INSERT INTO t SET col = expr, ... - stored as + // Columns plus one Values row; SetSyntax keeps the source form. + if len(columns) > 0 { + return nil, p.expectedError("VALUES or SELECT after a column list") + } + p.advance() // Consume SET + row := make([]ast.Expression, 0) + for { + if !p.isIdentifier() { + return nil, p.expectedError("column name") + } + columns = append(columns, &ast.Identifier{Name: p.currentToken.Token.Value}) + p.advance() + if !p.isType(models.TokenTypeEq) { + return nil, p.expectedError("=") + } + p.advance() // Consume = + expr, err := p.parseExpression() + if err != nil { + return nil, err + } + row = append(row, expr) + if !p.isType(models.TokenTypeComma) { + break + } + p.advance() // Consume comma + } + values = [][]ast.Expression{row} + setSyntax = true default: return nil, p.expectedError("VALUES, SELECT, or WITH") } @@ -209,7 +250,7 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { // Parse RETURNING clause if present (PostgreSQL) var returning []ast.Expression - if p.isType(models.TokenTypeReturning) || p.currentToken.Token.Value == "RETURNING" { + if p.isReturningKeyword() { p.advance() // Consume RETURNING var err error returning, err = p.parseReturningColumns() @@ -239,9 +280,51 @@ func (p *Parser) parseInsertStatement() (ast.Statement, error) { OnConflict: onConflict, OnDuplicateKey: onDuplicateKey, Returning: returning, + DefaultValues: defaultValues, + SetSyntax: setSyntax, }, nil } +// isInsertQueryParenStart reports whether the current '(' opens a +// parenthesized query source - INSERT INTO t [(cols)] (SELECT ...) - rather +// than a column list. A column list never starts with SELECT, WITH or '('. +func (p *Parser) isInsertQueryParenStart() bool { + if !p.isType(models.TokenTypeLParen) { + return false + } + switch p.peekToken().Token.Type { + case models.TokenTypeSelect, models.TokenTypeWith, models.TokenTypeLParen: + return true + } + return false +} + +// parseInsertSourceQuery parses the query source of INSERT ... SELECT, +// optionally wrapped in parentheses. RETURNING is not accepted as an implicit +// alias inside the source, so INSERT ... SELECT ... FROM s RETURNING id keeps +// its RETURNING clause (#610). +func (p *Parser) parseInsertSourceQuery() (ast.Statement, error) { + open := 0 + for p.isType(models.TokenTypeLParen) { + p.advance() // Consume ( + open++ + } + prev := p.dmlReturning + p.dmlReturning = true + stmt, err := p.parseQuerySource() + p.dmlReturning = prev + if err != nil { + return nil, err + } + for ; open > 0; open-- { + if !p.isType(models.TokenTypeRParen) { + return nil, p.expectedError(")") + } + p.advance() // Consume ) + } + return stmt, nil +} + // parseReturningColumns parses the columns in a RETURNING clause // Supports: column names, *, qualified names (table.column), expressions func (p *Parser) parseReturningColumns() ([]ast.Expression, error) { diff --git a/pkg/sql/parser/dml_update.go b/pkg/sql/parser/dml_update.go index 51c8610c..7ada993c 100644 --- a/pkg/sql/parser/dml_update.go +++ b/pkg/sql/parser/dml_update.go @@ -20,6 +20,7 @@ package parser import ( "strings" + goerrors "github.com/ajitpratap0/GoSQLX/pkg/errors" "github.com/ajitpratap0/GoSQLX/pkg/models" "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" ) @@ -93,6 +94,19 @@ func (p *Parser) parseUpdateStatement() (ast.Statement, error) { p.advance() // Consume comma } + // PostgreSQL / SQL Server / SQLite: UPDATE t SET ... FROM s [, ...] (#610). + var from []ast.TableReference + if p.isType(models.TokenTypeFrom) { + p.advance() // Consume FROM + prev := p.dmlReturning + p.dmlReturning = true + from, err = p.parseUpdateFromList() + p.dmlReturning = prev + if err != nil { + return nil, err + } + } + // Parse WHERE clause if present var whereClause ast.Expression if p.isType(models.TokenTypeWhere) { @@ -127,7 +141,29 @@ func (p *Parser) parseUpdateStatement() (ast.Statement, error) { return &ast.UpdateStatement{ TableName: tableName, Assignments: updates, + From: from, Where: whereClause, Returning: returning, }, nil } + +// parseUpdateFromList parses the comma-separated table references of +// UPDATE ... FROM. JOINs are rejected: UpdateStatement has no join list. +func (p *Parser) parseUpdateFromList() ([]ast.TableReference, error) { + var refs []ast.TableReference + for { + ref, err := p.parseFromTableReference() + if err != nil { + return nil, err + } + refs = append(refs, ref) + if !p.isType(models.TokenTypeComma) { + break + } + p.advance() // Consume comma + } + if p.isJoinKeyword() { + return nil, goerrors.UnsupportedFeatureError("JOIN in UPDATE ... FROM", p.currentLocation(), "") + } + return refs, nil +} diff --git a/pkg/sql/parser/expressions_literal.go b/pkg/sql/parser/expressions_literal.go index 2b701a5c..4398e136 100644 --- a/pkg/sql/parser/expressions_literal.go +++ b/pkg/sql/parser/expressions_literal.go @@ -460,9 +460,20 @@ func (p *Parser) parseNotOperand() (ast.Expression, error) { return p.parseComparisonExpression() } -// parseSubquery parses a subquery (SELECT or WITH statement). -// Expects current token to be SELECT or WITH. +// parseSubquery parses a nested subquery (SELECT or WITH statement), such as +// an IN/EXISTS/scalar subquery. Expects current token to be SELECT or WITH. +// A nested scope never ends at a DML RETURNING clause, so the INSERT-source +// RETURNING stop is cleared for its duration (#610). func (p *Parser) parseSubquery() (ast.Statement, error) { + prev := p.dmlReturning + p.dmlReturning = false + defer func() { p.dmlReturning = prev }() + return p.parseQuerySource() +} + +// parseQuerySource parses a SELECT or WITH statement without changing the +// DML RETURNING context. +func (p *Parser) parseQuerySource() (ast.Statement, error) { if p.isType(models.TokenTypeWith) { // WITH statement handles its own token consumption return p.parseWithStatement() diff --git a/pkg/sql/parser/insert_grammar_gaps_test.go b/pkg/sql/parser/insert_grammar_gaps_test.go new file mode 100644 index 00000000..68a72f84 --- /dev/null +++ b/pkg/sql/parser/insert_grammar_gaps_test.go @@ -0,0 +1,728 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package parser + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" +) + +// grammarGapCase is one #610 parse case: sql parsed in dialect must render +// as wantSQL, and wantSQL must re-parse to the same SQL() (fixpoint). +type grammarGapCase struct { + name string + dialect string + sql string + wantSQL string + check func(t *testing.T, stmt ast.Statement) +} + +// parseGapSingle parses sql in dialect and returns its only statement. The +// caller must release the returned AST. +func parseGapSingle(t *testing.T, sql, dialect string) (ast.Statement, *ast.AST) { + t.Helper() + tree, err := parseInsertCTEWithDialect(t, sql, dialect) + if err != nil { + if tree != nil { + ast.ReleaseAST(tree) + } + t.Fatalf("parse(%q, dialect %q): %v", sql, dialect, err) + } + if len(tree.Statements) != 1 { + n := len(tree.Statements) + ast.ReleaseAST(tree) + t.Fatalf("parse(%q, dialect %q): expected 1 statement, got %d", sql, dialect, n) + } + return tree.Statements[0], tree +} + +func runGrammarGapCases(t *testing.T, cases []grammarGapCase) { + t.Helper() + for _, tc := range cases { + t.Run(tc.name+"/"+dialectLabel(tc.dialect), func(t *testing.T) { + stmt, tree := parseGapSingle(t, tc.sql, tc.dialect) + defer ast.ReleaseAST(tree) + if tc.check != nil { + tc.check(t, stmt) + } + got := tree.SQL() + if got != tc.wantSQL { + t.Fatalf("SQL() = %q, want %q", got, tc.wantSQL) + } + _, tree2 := parseGapSingle(t, got, tc.dialect) + defer ast.ReleaseAST(tree2) + if again := tree2.SQL(); again != got { + t.Fatalf("re-parse SQL() = %q, want fixpoint %q", again, got) + } + }) + } +} + +func dialectLabel(d string) string { + if d == "" { + return "generic" + } + return d +} + +func asInsert(t *testing.T, stmt ast.Statement) *ast.InsertStatement { + t.Helper() + ins, ok := stmt.(*ast.InsertStatement) + if !ok { + t.Fatalf("expected *ast.InsertStatement, got %T", stmt) + } + return ins +} + +func asUpdate(t *testing.T, stmt ast.Statement) *ast.UpdateStatement { + t.Helper() + upd, ok := stmt.(*ast.UpdateStatement) + if !ok { + t.Fatalf("expected *ast.UpdateStatement, got %T", stmt) + } + return upd +} + +// insertSelectSource returns the INSERT's query source as a SelectStatement. +func insertSelectSource(t *testing.T, ins *ast.InsertStatement) *ast.SelectStatement { + t.Helper() + sel, ok := ins.Query.(*ast.SelectStatement) + if !ok { + t.Fatalf("expected Query *ast.SelectStatement, got %T", ins.Query) + } + return sel +} + +func wantReturning(n int) func(t *testing.T, stmt ast.Statement) { + return func(t *testing.T, stmt ast.Statement) { + t.Helper() + ins := asInsert(t, stmt) + if len(ins.Returning) != n { + t.Fatalf("len(Returning) = %d, want %d", len(ins.Returning), n) + } + } +} + +// fromUnaliasedReturning checks the #610 RETURNING case: the FROM table s +// keeps no alias and RETURNING has one item. +func fromUnaliasedReturning(t *testing.T, stmt ast.Statement) { + t.Helper() + ins := asInsert(t, stmt) + sel := insertSelectSource(t, ins) + if len(sel.From) != 1 || sel.From[0].Name != "s" || sel.From[0].Alias != "" { + t.Fatalf("FROM = %+v, want single unaliased table s", sel.From) + } + if len(ins.Returning) != 1 { + t.Fatalf("len(Returning) = %d, want 1", len(ins.Returning)) + } +} + +func parenSource(wantCols int) func(t *testing.T, stmt ast.Statement) { + return func(t *testing.T, stmt ast.Statement) { + t.Helper() + ins := asInsert(t, stmt) + if len(ins.Columns) != wantCols { + t.Fatalf("len(Columns) = %d, want %d", len(ins.Columns), wantCols) + } + if ins.Query == nil { + t.Fatal("Query is nil, want the parenthesized source") + } + if len(ins.Values) != 0 { + t.Fatalf("len(Values) = %d, want 0", len(ins.Values)) + } + } +} + +func TestInsertGrammarGaps_ReturningAfterSelect(t *testing.T) { + runGrammarGapCases(t, []grammarGapCase{ + { + name: "select star from s returning id", dialect: "postgresql", + sql: "INSERT INTO t SELECT * FROM s RETURNING id", + wantSQL: "INSERT INTO t SELECT * FROM s RETURNING id", + check: fromUnaliasedReturning, + }, + { + name: "column list returning star", dialect: "postgresql", + sql: "INSERT INTO t (a) SELECT b FROM s RETURNING *", + wantSQL: "INSERT INTO t (a) SELECT b FROM s RETURNING *", + check: func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + fromUnaliasedReturning(t, stmt) + id, ok := ins.Returning[0].(*ast.Identifier) + if !ok || id.Name != "*" { + t.Fatalf("Returning[0] = %#v, want Identifier *", ins.Returning[0]) + } + }, + }, + { + name: "lowercase select source", dialect: "postgresql", + sql: "insert into t select * from s returning id", + wantSQL: "INSERT INTO t SELECT * FROM s RETURNING id", + check: fromUnaliasedReturning, + }, + { + name: "lowercase values source", dialect: "postgresql", + sql: "insert into t (a) values (1) returning a", + wantSQL: "INSERT INTO t (a) VALUES (1) RETURNING a", + check: wantReturning(1), + }, + { + name: "union source", dialect: "postgresql", + sql: "INSERT INTO t SELECT a FROM s UNION SELECT b FROM u RETURNING a", + wantSQL: "INSERT INTO t SELECT a FROM s UNION SELECT b FROM u RETURNING a", + check: func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + if _, ok := ins.Query.(*ast.SetOperation); !ok { + t.Fatalf("Query = %T, want *ast.SetOperation", ins.Query) + } + if len(ins.Returning) != 1 { + t.Fatalf("len(Returning) = %d, want 1", len(ins.Returning)) + } + }, + }, + { + name: "cross join source", dialect: "postgresql", + sql: "INSERT INTO t SELECT a FROM s CROSS JOIN u RETURNING a", + wantSQL: "INSERT INTO t SELECT a FROM s CROSS JOIN u RETURNING a", + check: func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + sel := insertSelectSource(t, ins) + if len(sel.Joins) != 1 || sel.Joins[0].Right.Name != "u" || sel.Joins[0].Right.Alias != "" { + t.Fatalf("Joins = %+v, want one join to unaliased u", sel.Joins) + } + if len(ins.Returning) != 1 { + t.Fatalf("len(Returning) = %d, want 1", len(ins.Returning)) + } + }, + }, + { + name: "from-less select source", dialect: "postgresql", + sql: "INSERT INTO t SELECT 1 RETURNING id", + wantSQL: "INSERT INTO t SELECT 1 RETURNING id", + check: wantReturning(1), + }, + { + name: "leading cte", dialect: "postgresql", + sql: "WITH c AS (SELECT 1 AS a) INSERT INTO t SELECT a FROM c RETURNING a", + wantSQL: "WITH c AS (SELECT 1 AS a) INSERT INTO t SELECT a FROM c RETURNING a", + check: wantReturning(1), + }, + { + name: "explicit alias control", dialect: "postgresql", + sql: "INSERT INTO t SELECT * FROM s AS x RETURNING id", + wantSQL: "INSERT INTO t SELECT * FROM s x RETURNING id", + check: func(t *testing.T, stmt ast.Statement) { + sel := insertSelectSource(t, asInsert(t, stmt)) + if len(sel.From) != 1 || sel.From[0].Alias != "x" { + t.Fatalf("FROM = %+v, want alias x", sel.From) + } + }, + }, + { + name: "on conflict then returning", dialect: "postgresql", + sql: "INSERT INTO t SELECT * FROM s ON CONFLICT DO NOTHING RETURNING id", + wantSQL: "INSERT INTO t SELECT * FROM s ON CONFLICT DO NOTHING RETURNING id", + check: func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + if ins.OnConflict == nil { + t.Fatal("OnConflict is nil") + } + fromUnaliasedReturning(t, stmt) + }, + }, + }) +} + +func TestInsertGrammarGaps_DefaultValues(t *testing.T) { + check := func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + if !ins.DefaultValues { + t.Fatal("DefaultValues = false, want true") + } + if len(ins.Columns) != 0 || len(ins.Values) != 0 || ins.Query != nil { + t.Fatalf("Columns=%d Values=%d Query=%v, want all empty", len(ins.Columns), len(ins.Values), ins.Query) + } + } + var cases []grammarGapCase + for _, d := range []string{"", "postgresql", "sqlserver", "sqlite"} { + cases = append(cases, grammarGapCase{ + name: "default values", dialect: d, + sql: "INSERT INTO t DEFAULT VALUES", + wantSQL: "INSERT INTO t DEFAULT VALUES", + check: check, + }) + } + cases = append(cases, + grammarGapCase{ + name: "default values returning", dialect: "postgresql", + sql: "INSERT INTO t DEFAULT VALUES RETURNING id", + wantSQL: "INSERT INTO t DEFAULT VALUES RETURNING id", + check: func(t *testing.T, stmt ast.Statement) { + check(t, stmt) + wantReturning(1)(t, stmt) + }, + }, + grammarGapCase{ + name: "default values on conflict", dialect: "postgresql", + sql: "INSERT INTO t DEFAULT VALUES ON CONFLICT DO NOTHING", + wantSQL: "INSERT INTO t DEFAULT VALUES ON CONFLICT DO NOTHING", + check: check, + }, + ) + runGrammarGapCases(t, cases) +} + +func TestInsertGrammarGaps_SetSyntax(t *testing.T) { + check := func(wantCells int) func(t *testing.T, stmt ast.Statement) { + return func(t *testing.T, stmt ast.Statement) { + ins := asInsert(t, stmt) + if !ins.SetSyntax { + t.Fatal("SetSyntax = false, want true") + } + if len(ins.Columns) != wantCells { + t.Fatalf("len(Columns) = %d, want %d", len(ins.Columns), wantCells) + } + for i, want := range []string{"a", "b"}[:wantCells] { + id, ok := ins.Columns[i].(*ast.Identifier) + if !ok || id.Name != want { + t.Fatalf("Columns[%d] = %#v, want Identifier %s", i, ins.Columns[i], want) + } + } + if len(ins.Values) != 1 || len(ins.Values[0]) != wantCells { + t.Fatalf("Values = %v, want one row of %d cells", ins.Values, wantCells) + } + } + } + runGrammarGapCases(t, []grammarGapCase{ + { + name: "set", dialect: "mysql", + sql: "INSERT INTO t SET a = 1, b = 2", + wantSQL: "INSERT INTO t SET a = 1, b = 2", + check: check(2), + }, + { + name: "set", dialect: "mariadb", + sql: "INSERT INTO t SET a = 1, b = 2", + wantSQL: "INSERT INTO t SET a = 1, b = 2", + check: check(2), + }, + { + name: "set string", dialect: "mariadb", + sql: "INSERT INTO t SET a = 1, b = 'x'", + wantSQL: "INSERT INTO t SET a = 1, b = 'x'", + check: check(2), + }, + }) +} + +func TestInsertGrammarGaps_ParenthesizedSource(t *testing.T) { + var cases []grammarGapCase + for _, d := range []string{"", "postgresql", "mysql"} { + cases = append(cases, grammarGapCase{ + name: "paren select", dialect: d, + sql: "INSERT INTO t (SELECT * FROM s)", + wantSQL: "INSERT INTO t SELECT * FROM s", + check: parenSource(0), + }) + } + cases = append(cases, + grammarGapCase{ + name: "column list then paren select", dialect: "postgresql", + sql: "INSERT INTO t (a, b) (SELECT x, y FROM s)", + wantSQL: "INSERT INTO t (a, b) SELECT x, y FROM s", + check: parenSource(2), + }, + grammarGapCase{ + name: "double paren select", dialect: "", + sql: "INSERT INTO t ((SELECT * FROM s))", + wantSQL: "INSERT INTO t SELECT * FROM s", + check: parenSource(0), + }, + grammarGapCase{ + name: "paren union", dialect: "", + sql: "INSERT INTO t (SELECT a FROM s UNION SELECT b FROM u)", + wantSQL: "INSERT INTO t SELECT a FROM s UNION SELECT b FROM u", + check: parenSource(0), + }, + grammarGapCase{ + name: "paren with", dialect: "", + sql: "INSERT INTO t (WITH c AS (SELECT 1) SELECT * FROM c)", + wantSQL: "INSERT INTO t WITH c AS (SELECT 1) SELECT * FROM c", + check: parenSource(0), + }, + grammarGapCase{ + name: "paren select returning", dialect: "postgresql", + sql: "INSERT INTO t (SELECT * FROM s) RETURNING id", + wantSQL: "INSERT INTO t SELECT * FROM s RETURNING id", + check: func(t *testing.T, stmt ast.Statement) { + parenSource(0)(t, stmt) + fromUnaliasedReturning(t, stmt) + }, + }, + ) + runGrammarGapCases(t, cases) +} + +func TestUpdateFrom(t *testing.T) { + fromNames := func(want ...string) func(t *testing.T, stmt ast.Statement) { + return func(t *testing.T, stmt ast.Statement) { + upd := asUpdate(t, stmt) + if len(upd.From) != len(want) { + t.Fatalf("len(From) = %d, want %d", len(upd.From), len(want)) + } + for i, name := range want { + if upd.From[i].Name != name || upd.From[i].Alias != "" { + t.Fatalf("From[%d] = %+v, want unaliased %s", i, upd.From[i], name) + } + } + } + } + runGrammarGapCases(t, []grammarGapCase{ + { + name: "from where", dialect: "postgresql", + sql: "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + wantSQL: "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + check: fromNames("s"), + }, + { + name: "from list returning", dialect: "postgresql", + sql: "UPDATE t SET a = s.b FROM s, u WHERE t.id = s.id RETURNING t.a", + wantSQL: "UPDATE t SET a = s.b FROM s, u WHERE t.id = s.id RETURNING t.a", + check: fromNames("s", "u"), + }, + { + name: "from returning", dialect: "postgresql", + sql: "UPDATE t SET a = s.b FROM s RETURNING id", + wantSQL: "UPDATE t SET a = s.b FROM s RETURNING id", + check: fromNames("s"), + }, + { + name: "from where", dialect: "sqlserver", + sql: "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + wantSQL: "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", + check: fromNames("s"), + }, + }) +} + +func TestInsertGrammarGapsErrors(t *testing.T) { + tests := []struct { + name string + dialect string + sql string + wantErr string + }{ + {"set outside mysql", "postgresql", "INSERT INTO t SET a = 1", "VALUES, SELECT, or WITH"}, + {"set after column list", "mysql", "INSERT INTO t (a) SET a = 1", ""}, + {"default values after column list", "postgresql", "INSERT INTO t (a) DEFAULT VALUES", "DEFAULT VALUES cannot follow a column list"}, + {"bare default", "", "INSERT INTO t1 (a) DEFAULT", "VALUES, SELECT, or WITH"}, + {"unclosed paren source", "", "INSERT INTO t (SELECT * FROM s", ")"}, + {"paren non-query source", "", "INSERT INTO t (a) (1)", "VALUES, SELECT, or WITH"}, + {"update from join", "postgresql", "UPDATE t SET a = s.b FROM s JOIN u ON s.id = u.id WHERE t.id = s.id", "JOIN in UPDATE ... FROM"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree, err := parseInsertCTEWithDialect(t, tt.sql, tt.dialect) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil { + t.Fatalf("parse(%q, %q): expected error, got nil", tt.sql, tt.dialect) + } + if tt.wantErr != "" && !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("parse(%q, %q) error = %q, want it to contain %q", tt.sql, tt.dialect, err.Error(), tt.wantErr) + } + }) + } +} + +// selectFromAlias parses sql as a SELECT and returns FROM[0].Alias. +func selectFromAlias(t *testing.T, p *Parser, sql, dialect string) string { + t.Helper() + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + if dialect != "" { + tkz.SetDialect(keywords.SQLDialect(dialect)) + } + tokens, err := tkz.Tokenize([]byte(sql)) + if err != nil { + t.Fatalf("Tokenize(%q): %v", sql, err) + } + tree, err := p.ParseFromModelTokens(tokens) + if err != nil { + t.Fatalf("parse(%q, %q): %v", sql, dialect, err) + } + defer ast.ReleaseAST(tree) + if len(tree.Statements) != 1 { + t.Fatalf("expected 1 statement, got %d", len(tree.Statements)) + } + sel, ok := tree.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("expected *ast.SelectStatement, got %T", tree.Statements[0]) + } + if len(sel.From) != 1 { + t.Fatalf("len(From) = %d, want 1", len(sel.From)) + } + return sel.From[0].Alias +} + +// TestReturningAliasOutsideDML pins that RETURNING stays a valid implicit +// alias outside an INSERT source or UPDATE ... FROM list (#610). +func TestReturningAliasOutsideDML(t *testing.T) { + for _, d := range []string{"", "postgresql"} { + t.Run("bare/"+dialectLabel(d), func(t *testing.T) { + p := NewParser(WithDialect(d)) + defer p.Release() + if got := selectFromAlias(t, p, "SELECT * FROM s returning", d); got != "returning" { + t.Fatalf("Alias = %q, want %q", got, "returning") + } + }) + } + t.Run("as returning re-parses", func(t *testing.T) { + _, tree := parseGapSingle(t, "SELECT * FROM s AS returning", "postgresql") + defer ast.ReleaseAST(tree) + _, tree2 := parseGapSingle(t, tree.SQL(), "postgresql") + ast.ReleaseAST(tree2) + }) + t.Run("quoted", func(t *testing.T) { + p := NewParser() + defer p.Release() + if got := selectFromAlias(t, p, `SELECT * FROM s "RETURNING"`, ""); got != "RETURNING" { + t.Fatalf("Alias = %q, want %q", got, "RETURNING") + } + }) +} + +// TestDMLReturningFlagRestored pins that the INSERT-source context flag is +// restored on error paths and cleared by Reset (#610). +func TestDMLReturningFlagRestored(t *testing.T) { + const dialect = "postgresql" + parseErr := func(p *Parser, sql string) { + t.Helper() + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + tkz.SetDialect(keywords.SQLDialect(dialect)) + tokens, err := tkz.Tokenize([]byte(sql)) + if err != nil { + t.Fatalf("Tokenize(%q): %v", sql, err) + } + tree, err := p.ParseFromModelTokens(tokens) + if tree != nil { + ast.ReleaseAST(tree) + } + if err == nil { + t.Fatalf("parse(%q): expected error", sql) + } + } + + t.Run("same instance", func(t *testing.T) { + p := NewParser(WithDialect(dialect)) + defer p.Release() + parseErr(p, "INSERT INTO t SELECT * FROM (") + if p.dmlReturning { + t.Fatal("dmlReturning still set after a failed INSERT source parse") + } + if got := selectFromAlias(t, p, "SELECT * FROM s returning", dialect); got != "returning" { + t.Fatalf("Alias = %q, want %q", got, "returning") + } + }) + t.Run("update from error", func(t *testing.T) { + p := NewParser(WithDialect(dialect)) + defer p.Release() + parseErr(p, "UPDATE t SET a = 1 FROM (") + if p.dmlReturning { + t.Fatal("dmlReturning still set after a failed UPDATE ... FROM parse") + } + }) + t.Run("reset clears", func(t *testing.T) { + p := GetParser() + p.dmlReturning = true + PutParser(p) + if p.dmlReturning { + t.Fatal("Reset did not clear dmlReturning") + } + p2 := GetParser() + defer PutParser(p2) + p2.ApplyOptions(WithDialect(dialect)) + if got := selectFromAlias(t, p2, "SELECT * FROM s returning", dialect); got != "returning" { + t.Fatalf("Alias = %q, want %q", got, "returning") + } + }) +} + +// TestDMLReturningQuotedAlias pins that a backtick- or bracket-quoted alias +// named returning stays an alias inside an INSERT source or UPDATE ... FROM +// list. Quoting is the escape hatch for keyword-named identifiers, so only an +// unquoted RETURNING may start the DML RETURNING clause (#610 review). +func TestDMLReturningQuotedAlias(t *testing.T) { + insertFromAlias := func(wantReturning int) func(t *testing.T, stmt ast.Statement) { + return func(t *testing.T, stmt ast.Statement) { + t.Helper() + ins := asInsert(t, stmt) + sel := insertSelectSource(t, ins) + if len(sel.From) != 1 || sel.From[0].Name != "s" || sel.From[0].Alias != "returning" { + t.Fatalf("FROM = %+v, want s aliased returning", sel.From) + } + if len(ins.Returning) != wantReturning { + t.Fatalf("len(Returning) = %d, want %d", len(ins.Returning), wantReturning) + } + } + } + insertJoinAlias := func(t *testing.T, stmt ast.Statement) { + t.Helper() + sel := insertSelectSource(t, asInsert(t, stmt)) + if len(sel.Joins) != 1 { + t.Fatalf("len(Joins) = %d, want 1", len(sel.Joins)) + } + if got := sel.Joins[0].Right.Alias; got != "returning" { + t.Fatalf("join alias = %q, want %q", got, "returning") + } + } + updateFromAlias := func(t *testing.T, stmt ast.Statement) { + t.Helper() + upd := asUpdate(t, stmt) + if len(upd.From) != 1 || upd.From[0].Name != "s" || upd.From[0].Alias != "returning" { + t.Fatalf("From = %+v, want s aliased returning", upd.From) + } + } + nestedAlias := func(t *testing.T, stmt ast.Statement) { + t.Helper() + sel := insertSelectSource(t, asInsert(t, stmt)) + if len(sel.From) != 1 || sel.From[0].Subquery == nil { + t.Fatalf("FROM = %+v, want one derived table", sel.From) + } + inner := sel.From[0].Subquery + if len(inner.From) != 1 || inner.From[0].Alias != "returning" { + t.Fatalf("inner FROM = %+v, want s aliased returning", inner.From) + } + } + + cases := []struct { + name, dialect, sql string + check func(t *testing.T, stmt ast.Statement) + }{ + {"insert from alias where", "mysql", "INSERT INTO t SELECT * FROM s `returning` WHERE `returning`.a = 1", insertFromAlias(0)}, + {"insert from alias where", "mariadb", "INSERT INTO t SELECT * FROM s `returning` WHERE `returning`.a = 1", insertFromAlias(0)}, + {"insert from alias eof", "mysql", "INSERT INTO t SELECT * FROM s `returning`", insertFromAlias(0)}, + {"insert from alias where", "sqlserver", "INSERT INTO t SELECT * FROM s [returning] WHERE [returning].id = 1", insertFromAlias(0)}, + {"insert from alias then returning", "postgresql", "INSERT INTO t SELECT * FROM s `returning` RETURNING id", insertFromAlias(1)}, + {"insert join alias", "mysql", "INSERT INTO t SELECT * FROM s JOIN r `returning` ON s.id = `returning`.id", insertJoinAlias}, + {"insert join alias", "sqlserver", "INSERT INTO t SELECT * FROM s JOIN r [returning] ON s.id = [returning].id", insertJoinAlias}, + {"insert nested subquery alias", "mysql", "INSERT INTO t SELECT * FROM (SELECT * FROM s `returning`) x", nestedAlias}, + {"update from alias", "sqlserver", "UPDATE t SET a = 1 FROM s [returning] WHERE t.id = [returning].id", updateFromAlias}, + {"update from alias", "postgresql", "UPDATE t SET a = 1 FROM s `returning` WHERE t.id = `returning`.id", updateFromAlias}, + } + for _, tc := range cases { + t.Run(tc.name+"/"+dialectLabel(tc.dialect), func(t *testing.T) { + stmt, tree := parseGapSingle(t, tc.sql, tc.dialect) + defer ast.ReleaseAST(tree) + tc.check(t, stmt) + }) + } +} + +// TestDMLReturningNestedScopes pins that the RETURNING stop applies only to the +// outermost INSERT source query: nested derived tables, subqueries and CTE +// bodies, and aliases that cannot start a RETURNING clause, keep parsing +// "returning" as an ordinary identifier (#610 review). +func TestDMLReturningNestedScopes(t *testing.T) { + cases := []struct { + name string + sql string + }{ + {"cte body", "INSERT INTO t WITH c AS (SELECT * FROM s returning) SELECT * FROM c"}, + {"in subquery", "INSERT INTO t SELECT a FROM s WHERE a IN (SELECT b FROM r returning)"}, + {"exists subquery", "INSERT INTO t SELECT a FROM s WHERE EXISTS (SELECT 1 FROM r returning)"}, + {"scalar subquery", "INSERT INTO t SELECT (SELECT max(b) FROM r returning) FROM s"}, + {"derived table", "INSERT INTO t SELECT a FROM (SELECT a FROM r) returning"}, + {"join alias on", "INSERT INTO t SELECT * FROM s JOIN r returning ON s.id = returning.id"}, + {"alias then left join", "INSERT INTO t SELECT * FROM s returning LEFT JOIN r ON s.id = r.id"}, + {"alias then right join", "INSERT INTO t SELECT * FROM s returning RIGHT JOIN r ON s.id = r.id"}, + {"alias then left outer join", "INSERT INTO t SELECT * FROM s returning LEFT OUTER JOIN r ON s.id = r.id"}, + {"alias then offset", "INSERT INTO t SELECT * FROM s returning OFFSET 1"}, + {"alias then fetch", "INSERT INTO t SELECT * FROM s returning FETCH FIRST 1 ROWS ONLY"}, + {"alias then window", "INSERT INTO t SELECT * FROM s returning WINDOW w AS (PARTITION BY a)"}, + {"alias then for update", "INSERT INTO t SELECT * FROM s returning FOR UPDATE"}, + {"update from subquery", "UPDATE t SET a = 1 FROM (SELECT a FROM r returning) x WHERE t.id = x.a"}, + {"update from alias then where", "UPDATE t SET a = 1 FROM s returning WHERE t.id = returning.id"}, + } + for _, d := range []string{"mysql", "sqlserver", "snowflake", "clickhouse"} { + for _, tc := range cases { + t.Run(tc.name+"/"+d, func(t *testing.T) { + tree, err := parseInsertCTEWithDialect(t, tc.sql, d) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err != nil { + t.Fatalf("parse(%q, %s): %v", tc.sql, d, err) + } + }) + } + } + + // A bare returning alias followed by a dialect-specific clause that may + // follow a table reference keeps parsing as an alias, as it does without + // RETURNING support. + for _, tc := range []struct{ dialect, sql string }{ + {"snowflake", "INSERT INTO t SELECT a, ROW_NUMBER() OVER (ORDER BY a) rn FROM s returning QUALIFY rn = 1"}, + {"snowflake", "INSERT INTO t SELECT * FROM s returning PIVOT (SUM(a) FOR b IN ('x', 'y')) p"}, + {"sqlserver", "INSERT INTO t SELECT * FROM s returning PIVOT (SUM(a) FOR b IN ('x', 'y')) p"}, + {"oracle", "INSERT INTO t SELECT * FROM s returning UNPIVOT (v FOR b IN (x, y)) p"}, + {"sqlserver", "INSERT INTO t SELECT * FROM s returning WITH (NOLOCK)"}, + {"clickhouse", "INSERT INTO t SELECT * FROM s returning PREWHERE a = 1"}, + {"clickhouse", "INSERT INTO t SELECT * FROM s returning FINAL"}, + {"clickhouse", "INSERT INTO t SELECT * FROM s returning SAMPLE 0.1"}, + {"clickhouse", "INSERT INTO t SELECT * FROM s returning ARRAY JOIN arr"}, + {"clickhouse", "INSERT INTO t SELECT * FROM s returning LEFT ARRAY JOIN arr"}, + {"oracle", "INSERT INTO t SELECT * FROM s returning START WITH a = 1 CONNECT BY PRIOR id = parent"}, + {"oracle", "INSERT INTO t SELECT * FROM s returning CONNECT BY PRIOR id = parent"}, + {"mariadb", "INSERT INTO t SELECT * FROM s returning START WITH a = 1 CONNECT BY PRIOR id = parent"}, + } { + t.Run("dialect clause/"+tc.dialect+"/"+tc.sql, func(t *testing.T) { + tree, err := parseInsertCTEWithDialect(t, tc.sql, tc.dialect) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err != nil { + t.Fatalf("parse(%q, %s): %v", tc.sql, tc.dialect, err) + } + }) + } + + // The outermost RETURNING clause is still recognised. + for _, sql := range []string{ + "INSERT INTO t SELECT a FROM s WHERE a IN (SELECT b FROM r) RETURNING id", + "INSERT INTO t SELECT a FROM s RETURNING ARRAY[a, 1]", + "INSERT INTO t SELECT a FROM s RETURNING window_id", + "INSERT INTO t WITH c AS (SELECT * FROM s) SELECT * FROM c RETURNING id", + "INSERT INTO t SELECT a FROM (SELECT a FROM r) d RETURNING id", + } { + stmt, tree := parseGapSingle(t, sql, "postgresql") + ins, ok := stmt.(*ast.InsertStatement) + if !ok { + ast.ReleaseAST(tree) + t.Fatalf("parse(%q): got %T", sql, stmt) + } + if len(ins.Returning) != 1 { + ast.ReleaseAST(tree) + t.Fatalf("parse(%q): Returning = %d, want 1", sql, len(ins.Returning)) + } + ast.ReleaseAST(tree) + } +} diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 14cf21eb..3ea2098b 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -143,6 +143,7 @@ func (p *Parser) Reset() { // Reset clears both fields in lockstep; see WithDialect for the active path. p.dialectTyped = dialect.Unknown p.capabilitiesCache = dialect.Capabilities{} + p.dmlReturning = false } // currentLocation returns the source location of the current token. @@ -298,6 +299,10 @@ type Parser struct { // dialectTyped. It is refreshed alongside dialectTyped in WithDialect // and Reset so that Capabilities() is a single struct-field read. capabilitiesCache dialect.Capabilities + // dmlReturning is true while parsing an INSERT source query or an + // UPDATE ... FROM list, where an unquoted RETURNING starts the RETURNING + // clause and must not be taken as an implicit alias (#610). + dmlReturning bool } // Deprecated: Parse is provided for backward compatibility only and is scheduled for @@ -1207,12 +1212,88 @@ func (p *Parser) isSettingsKeyword() bool { p.isTokenMatch("SETTINGS") } +// isReturningKeyword reports whether the current token is an unquoted +// RETURNING keyword (any case). Quoted identifiers named returning are names, +// not the keyword: quoting is the only way to use a keyword as an identifier. +func (p *Parser) isReturningKeyword() bool { + if p.isType(models.TokenTypeReturning) { + return true + } + return p.isType(models.TokenTypeIdentifier) && + strings.EqualFold(p.currentToken.Token.Value, "RETURNING") && + !p.isQuotedIdentifierToken() +} + +// isQuotedIdentifierToken reports whether the current identifier token was +// written with delimiters. Bracket identifiers record Token.Quote = '['; the +// tokenizer leaves Quote zero for backtick identifiers, so a source span wider +// than the value also counts as quoted. Tokens without spans (Start == End) +// are treated as unquoted. +func (p *Parser) isQuotedIdentifierToken() bool { + tok := p.currentToken + if tok.Token.Quote != 0 { + return true + } + return tok.Start.Line == tok.End.Line && tok.End.Column-tok.Start.Column > len(tok.Token.Value) +} + +// isDMLReturningKeyword reports whether the current token is RETURNING inside +// an INSERT source query or UPDATE ... FROM list, where it starts the +// statement's RETURNING clause rather than naming an alias (#610). +// +// A RETURNING list cannot begin with a join keyword (including ON and USING), +// a clause keyword, a comma, a dot, a closing parenthesis or the end of the +// statement, nor with any clause that may follow a table reference in the +// current dialect, so when one of those follows, the token is an ordinary +// alias (FROM s JOIN r returning ON ..., FROM s returning LEFT JOIN ...), as +// it is in dialects that do not reserve the word. +func (p *Parser) isDMLReturningKeyword() bool { + if !p.dmlReturning || !p.isReturningKeyword() { + return false + } + next := p.peekToken().Token.Type + if next.IsJoinKeyword() { + return false + } + switch next { + case models.TokenTypeComma, models.TokenTypePeriod, models.TokenTypeRParen, + models.TokenTypeEOF, models.TokenTypeSemicolon, + models.TokenTypeWhere, models.TokenTypeGroup, models.TokenTypeHaving, + models.TokenTypeOrder, models.TokenTypeLimit, models.TokenTypeOffset, + models.TokenTypeFetch, models.TokenTypeFor, models.TokenTypeWith, + models.TokenTypeUnion, models.TokenTypeExcept, models.TokenTypeIntersect: + return false + } + return !p.nextStartsTableClause() +} + +// nextStartsTableClause reports whether the token after the current one +// starts a dialect-specific clause that may follow a table reference (WINDOW, +// QUALIFY, PIVOT, START WITH / CONNECT BY, SAMPLE, MATCH_RECOGNIZE, time +// travel, ClickHouse FINAL / PREWHERE / ARRAY JOIN / SETTINGS, MINUS). It +// evaluates the same dialect-gated predicates that guard FROM aliases, one +// token ahead, and restores the parser position before returning. +func (p *Parser) nextStartsTableClause() bool { + savedPos, savedTok := p.currentPos, p.currentToken + p.advance() + defer func() { p.currentPos, p.currentToken = savedPos, savedTok }() + if p.isWindowClauseKeyword() || p.isQualifyKeyword() || p.isMinusSetOp() || + p.isPivotKeyword() || p.isUnpivotKeyword() || p.isMariaDBClauseStart() || + p.isSampleKeyword() || p.isMatchRecognizeKeyword() || + p.isSnowflakeTimeTravelStart() || p.isSettingsKeyword() { + return true + } + return p.dialect == string(keywords.DialectClickHouse) && + (p.isTokenMatch("FINAL") || p.isTokenMatch("PREWHERE") || + (p.isType(models.TokenTypeArray) && p.peekToken().Token.Type == models.TokenTypeJoin)) +} + // canBeAlias checks if current token can be used as an alias. // Aliases can be IDENT, double-quoted identifiers, or certain non-reserved keywords, // but NOT contextual clause keywords that would be consumed as aliases by mistake // (e.g. MINUS in Snowflake/Oracle, QUALIFY in Snowflake/BigQuery, WINDOW). func (p *Parser) canBeAlias() bool { - if p.isMinusSetOp() || p.isQualifyKeyword() || p.isWindowClauseKeyword() || p.isSettingsKeyword() { + if p.isMinusSetOp() || p.isQualifyKeyword() || p.isWindowClauseKeyword() || p.isSettingsKeyword() || p.isDMLReturningKeyword() { return false } return p.isIdentifier() || p.isNonReservedKeyword() diff --git a/pkg/sql/parser/select_clauses.go b/pkg/sql/parser/select_clauses.go index edcc52d1..ac31b064 100644 --- a/pkg/sql/parser/select_clauses.go +++ b/pkg/sql/parser/select_clauses.go @@ -63,7 +63,8 @@ func (p *Parser) parseFromClause() (tableName string, tables []ast.TableReferenc !p.isType(models.TokenTypeSemicolon) && !p.isType(models.TokenTypeRParen) && !p.isAnyType(models.TokenTypeUnion, models.TokenTypeExcept, models.TokenTypeIntersect) && - !p.isMinusSetOp() { + !p.isMinusSetOp() && + !p.isDMLReturningKeyword() { return "", nil, nil, p.expectedError("FROM, semicolon, or end of statement") } return "", nil, nil, nil diff --git a/pkg/sql/parser/select_subquery.go b/pkg/sql/parser/select_subquery.go index 716ddcd0..096c59b5 100644 --- a/pkg/sql/parser/select_subquery.go +++ b/pkg/sql/parser/select_subquery.go @@ -182,7 +182,7 @@ func (p *Parser) parseFromTableReference() (ast.TableReference, error) { // Similarly, START followed by WITH is a hierarchical query seed, not an alias. // Don't consume PIVOT/UNPIVOT as a table alias — they are contextual // keywords in SQL Server/Oracle and must reach the pivot-clause parser below. - if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() { + if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() { if p.isType(models.TokenTypeAs) { p.advance() // Consume AS if !p.isIdentifier() { @@ -265,6 +265,11 @@ func (p *Parser) parseFromTableReference() (ast.TableReference, error) { // tables are bounded by MaxRecursionDepth. This path does not pass through // parseExpression, which guards other nesting. func (p *Parser) parseDerivedTableSelect() (ast.Statement, error) { + // A derived table is a nested scope: the INSERT-source RETURNING stop + // does not apply inside it (#610). + prev := p.dmlReturning + p.dmlReturning = false + defer func() { p.dmlReturning = prev }() err := p.enterRecursion() defer p.leaveRecursion() if err != nil { @@ -326,7 +331,7 @@ func (p *Parser) parseJoinedTableRef(joinType string) (ast.TableReference, error // Similarly, START followed by WITH is a hierarchical query seed, not an alias. // Don't consume PIVOT/UNPIVOT as a table alias — they are contextual // keywords in SQL Server/Oracle and must reach the pivot-clause parser below. - if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() { + if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() { if p.isType(models.TokenTypeAs) { p.advance() if !p.isIdentifier() {