Skip to content

Commit 04202f6

Browse files
committed
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
1 parent ecec179 commit 04202f6

9 files changed

Lines changed: 125 additions & 12 deletions

File tree

internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/db.go

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/models.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/ddl_alter_table_rename_column_multiple/mysql/go/query.sql.go

Lines changed: 37 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/* name: GetFoo :many */
2+
SELECT qux, quux FROM foo;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
CREATE TABLE foo (bar text, baz text);
2+
ALTER TABLE foo
3+
RENAME COLUMN bar TO qux,
4+
RENAME COLUMN baz TO quux;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"version": "1",
3+
"packages": [
4+
{
5+
"path": "go",
6+
"engine": "mysql",
7+
"name": "querytest",
8+
"schema": "schema.sql",
9+
"queries": "query.sql"
10+
}
11+
]
12+
}

internal/engine/dolphin/convert.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,13 @@ func (c *cc) convertAlterTableStmt(n *pcast.AlterTableStmt) ast.Node {
9595
// spew.Dump("add const", spec)
9696

9797
case pcast.AlterTableRenameColumn:
98-
// TODO: Returning here may be incorrect if there are multiple specs
9998
oldName := spec.OldColumnName.String()
10099
newName := spec.NewColumnName.String()
101-
return &ast.RenameColumnStmt{
102-
Table: parseTableName(n.Table),
103-
Col: &ast.ColumnRef{Name: oldName},
104-
NewName: &newName,
105-
}
100+
alt.Cmds.Items = append(alt.Cmds.Items, &ast.AlterTableCmd{
101+
Name: &oldName,
102+
Newname: &newName,
103+
Subtype: ast.AT_RenameColumn,
104+
})
106105

107106
case pcast.AlterTableRenameTable:
108107
// TODO: Returning here may be incorrect if there are multiple specs

internal/sql/ast/alter_table_cmd.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const (
88
AT_DropColumn
99
AT_DropNotNull
1010
AT_SetNotNull
11+
AT_RenameColumn
1112
)
1213

1314
type AlterTableType int
@@ -24,6 +25,8 @@ func (t AlterTableType) String() string {
2425
return "DropNotNull"
2526
case AT_SetNotNull:
2627
return "SetNotNull"
28+
case AT_RenameColumn:
29+
return "RenameColumn"
2730
default:
2831
return "Unknown"
2932
}
@@ -32,6 +35,7 @@ func (t AlterTableType) String() string {
3235
type AlterTableCmd struct {
3336
Subtype AlterTableType
3437
Name *string
38+
Newname *string
3539
Def *ColumnDef
3640
Newowner *RoleSpec
3741
Behavior DropBehavior

internal/sql/catalog/table.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ func isStmtImplemented(stmt *ast.AlterTableStmt) bool {
177177
implemented = true
178178
case ast.AT_SetNotNull:
179179
implemented = true
180+
case ast.AT_RenameColumn:
181+
implemented = true
180182
}
181183
}
182184
}
@@ -215,6 +217,10 @@ func (c *Catalog) alterTable(stmt *ast.AlterTableStmt) error {
215217
if err := table.setNotNull(cmd); err != nil {
216218
return err
217219
}
220+
case ast.AT_RenameColumn:
221+
if err := c.renameColumnOnTable(table, *cmd.Name, *cmd.Newname); err != nil {
222+
return err
223+
}
218224
}
219225
}
220226
}
@@ -394,22 +400,26 @@ func (c *Catalog) renameColumn(stmt *ast.RenameColumnStmt) error {
394400
if err != nil {
395401
return checkMissing(err, stmt.MissingOk)
396402
}
403+
return c.renameColumnOnTable(tbl, stmt.Col.Name, *stmt.NewName)
404+
}
405+
406+
func (c *Catalog) renameColumnOnTable(tbl *Table, oldName, newName string) error {
397407
idx := -1
398408
for i := range tbl.Columns {
399-
if tbl.Columns[i].Name == stmt.Col.Name {
409+
if tbl.Columns[i].Name == oldName {
400410
idx = i
401411
}
402-
if tbl.Columns[i].Name == *stmt.NewName {
403-
return sqlerr.ColumnExists(tbl.Rel.Name, *stmt.NewName)
412+
if tbl.Columns[i].Name == newName {
413+
return sqlerr.ColumnExists(tbl.Rel.Name, newName)
404414
}
405415
}
406416
if idx == -1 {
407-
return sqlerr.ColumnNotFound(tbl.Rel.Name, stmt.Col.Name)
417+
return sqlerr.ColumnNotFound(tbl.Rel.Name, oldName)
408418
}
409-
tbl.Columns[idx].Name = *stmt.NewName
419+
tbl.Columns[idx].Name = newName
410420

411421
if tbl.Columns[idx].linkedType {
412-
name := fmt.Sprintf("%s_%s", tbl.Rel.Name, *stmt.NewName)
422+
name := fmt.Sprintf("%s_%s", tbl.Rel.Name, newName)
413423
rename := &ast.RenameTypeStmt{
414424
Type: &tbl.Columns[idx].Type,
415425
NewName: &name,

0 commit comments

Comments
 (0)