From 04202f6ce32df124e76da7650aeed9aaf93705b9 Mon Sep 17 00:00:00 2001 From: Dimas Febri <149291944+dimasfeb19@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:21:03 +0700 Subject: [PATCH] Apply every RENAME COLUMN in a multi-column MySQL ALTER convertAlterTableStmt returned on the first RENAME COLUMN spec, so any further renames in the same ALTER TABLE were dropped and queries using the later columns failed to compile. Emit an AT_RenameColumn command per spec, like the other multi-spec actions, and apply it in the catalog. Fixes #4493 --- .../mysql/go/db.go | 31 ++++++++++++++++ .../mysql/go/models.go | 14 +++++++ .../mysql/go/query.sql.go | 37 +++++++++++++++++++ .../mysql/query.sql | 2 + .../mysql/schema.sql | 4 ++ .../mysql/sqlc.json | 12 ++++++ internal/engine/dolphin/convert.go | 11 +++--- internal/sql/ast/alter_table_cmd.go | 4 ++ internal/sql/catalog/table.go | 22 ++++++++--- 9 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/db.go create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/models.go create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/query.sql.go create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/query.sql create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/schema.sql create mode 100644 internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/sqlc.json diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/db.go b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/db.go new file mode 100644 index 0000000000..80dd6ab1f6 --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/db.go @@ -0,0 +1,31 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 + +package querytest + +import ( + "context" + "database/sql" +) + +type DBTX interface { + ExecContext(context.Context, string, ...interface{}) (sql.Result, error) + PrepareContext(context.Context, string) (*sql.Stmt, error) + QueryContext(context.Context, string, ...interface{}) (*sql.Rows, error) + QueryRowContext(context.Context, string, ...interface{}) *sql.Row +} + +func New(db DBTX) *Queries { + return &Queries{db: db} +} + +type Queries struct { + db DBTX +} + +func (q *Queries) WithTx(tx *sql.Tx) *Queries { + return &Queries{ + db: tx, + } +} diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/models.go b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/models.go new file mode 100644 index 0000000000..c9d6dca77c --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/models.go @@ -0,0 +1,14 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 + +package querytest + +import ( + "database/sql" +) + +type Foo struct { + Qux sql.NullString + Quux sql.NullString +} diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/query.sql.go b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/query.sql.go new file mode 100644 index 0000000000..1d31f3c448 --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/query.sql.go @@ -0,0 +1,37 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: query.sql + +package querytest + +import ( + "context" +) + +const getFoo = `-- name: GetFoo :many +SELECT qux, quux FROM foo +` + +func (q *Queries) GetFoo(ctx context.Context) ([]Foo, error) { + rows, err := q.db.QueryContext(ctx, getFoo) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Foo + for rows.Next() { + var i Foo + if err := rows.Scan(&i.Qux, &i.Quux); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/query.sql b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/query.sql new file mode 100644 index 0000000000..9ce3928084 --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/query.sql @@ -0,0 +1,2 @@ +/* name: GetFoo :many */ +SELECT qux, quux FROM foo; diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/schema.sql b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/schema.sql new file mode 100644 index 0000000000..6c34fc66f5 --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/schema.sql @@ -0,0 +1,4 @@ +CREATE TABLE foo (bar text, baz text); +ALTER TABLE foo + RENAME COLUMN bar TO qux, + RENAME COLUMN baz TO quux; diff --git a/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/sqlc.json b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/sqlc.json new file mode 100644 index 0000000000..e41c39e8b3 --- /dev/null +++ b/internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "path": "go", + "engine": "mysql", + "name": "querytest", + "schema": "schema.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/engine/dolphin/convert.go b/internal/engine/dolphin/convert.go index cfd83b5c4d..0d2ff31613 100644 --- a/internal/engine/dolphin/convert.go +++ b/internal/engine/dolphin/convert.go @@ -95,14 +95,13 @@ func (c *cc) convertAlterTableStmt(n *pcast.AlterTableStmt) ast.Node { // spew.Dump("add const", spec) case pcast.AlterTableRenameColumn: - // TODO: Returning here may be incorrect if there are multiple specs oldName := spec.OldColumnName.String() newName := spec.NewColumnName.String() - return &ast.RenameColumnStmt{ - Table: parseTableName(n.Table), - Col: &ast.ColumnRef{Name: oldName}, - NewName: &newName, - } + alt.Cmds.Items = append(alt.Cmds.Items, &ast.AlterTableCmd{ + Name: &oldName, + Newname: &newName, + Subtype: ast.AT_RenameColumn, + }) case pcast.AlterTableRenameTable: // TODO: Returning here may be incorrect if there are multiple specs diff --git a/internal/sql/ast/alter_table_cmd.go b/internal/sql/ast/alter_table_cmd.go index 90ffd891eb..3e42f57d0e 100644 --- a/internal/sql/ast/alter_table_cmd.go +++ b/internal/sql/ast/alter_table_cmd.go @@ -8,6 +8,7 @@ const ( AT_DropColumn AT_DropNotNull AT_SetNotNull + AT_RenameColumn ) type AlterTableType int @@ -24,6 +25,8 @@ func (t AlterTableType) String() string { return "DropNotNull" case AT_SetNotNull: return "SetNotNull" + case AT_RenameColumn: + return "RenameColumn" default: return "Unknown" } @@ -32,6 +35,7 @@ func (t AlterTableType) String() string { type AlterTableCmd struct { Subtype AlterTableType Name *string + Newname *string Def *ColumnDef Newowner *RoleSpec Behavior DropBehavior diff --git a/internal/sql/catalog/table.go b/internal/sql/catalog/table.go index dc30acfa1e..5c1bb8ea3f 100644 --- a/internal/sql/catalog/table.go +++ b/internal/sql/catalog/table.go @@ -177,6 +177,8 @@ func isStmtImplemented(stmt *ast.AlterTableStmt) bool { implemented = true case ast.AT_SetNotNull: implemented = true + case ast.AT_RenameColumn: + implemented = true } } } @@ -215,6 +217,10 @@ func (c *Catalog) alterTable(stmt *ast.AlterTableStmt) error { if err := table.setNotNull(cmd); err != nil { return err } + case ast.AT_RenameColumn: + if err := c.renameColumnOnTable(table, *cmd.Name, *cmd.Newname); err != nil { + return err + } } } } @@ -394,22 +400,26 @@ func (c *Catalog) renameColumn(stmt *ast.RenameColumnStmt) error { if err != nil { return checkMissing(err, stmt.MissingOk) } + return c.renameColumnOnTable(tbl, stmt.Col.Name, *stmt.NewName) +} + +func (c *Catalog) renameColumnOnTable(tbl *Table, oldName, newName string) error { idx := -1 for i := range tbl.Columns { - if tbl.Columns[i].Name == stmt.Col.Name { + if tbl.Columns[i].Name == oldName { idx = i } - if tbl.Columns[i].Name == *stmt.NewName { - return sqlerr.ColumnExists(tbl.Rel.Name, *stmt.NewName) + if tbl.Columns[i].Name == newName { + return sqlerr.ColumnExists(tbl.Rel.Name, newName) } } if idx == -1 { - return sqlerr.ColumnNotFound(tbl.Rel.Name, stmt.Col.Name) + return sqlerr.ColumnNotFound(tbl.Rel.Name, oldName) } - tbl.Columns[idx].Name = *stmt.NewName + tbl.Columns[idx].Name = newName if tbl.Columns[idx].linkedType { - name := fmt.Sprintf("%s_%s", tbl.Rel.Name, *stmt.NewName) + name := fmt.Sprintf("%s_%s", tbl.Rel.Name, newName) rename := &ast.RenameTypeStmt{ Type: &tbl.Columns[idx].Type, NewName: &name,