Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: check if the column name already exists when we rename the column name #6284

Merged
merged 7 commits into from
Apr 16, 2018
Merged

ddl: check if the column name already exists when we rename the column name #6284

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
77f6972
ddl: fix rename column name problem
zimulala Apr 13, 2018
ff6a539
Merge branch 'master' into zimuxia/rename-col
zimulala Apr 13, 2018
7a7b50a
ddl: merge master
zimulala Apr 13, 2018
39d3aa3
ddl: add a test
zimulala Apr 16, 2018
170e759
ddl: add comments
zimulala Apr 16, 2018
0ea27af
ddl: tiny clean
zimulala Apr 16, 2018
39048c3
Merge branch 'master' into rename-col
shenli Apr 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ddl/column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,14 @@ func (d *ddl) doModifyColumn(t *meta.Meta, job *model.Job, newCol *model.ColumnI
job.State = model.JobStateCancelled
return ver, infoschema.ErrColumnNotExists.GenByArgs(oldName, tblInfo.Name)
}
// If we want to rename the column name, we need to check whether it already exists.
if newCol.Name.L != oldName.L {
c := findCol(tblInfo.Columns, newCol.Name.L)
if c != nil {
job.State = model.JobStateCancelled
return ver, infoschema.ErrColumnExists.GenByArgs(newCol.Name)
}
}

// We need the latest column's offset and state. This information can be obtained from the store.
newCol.Offset = oldCol.Offset
Expand Down
10 changes: 9 additions & 1 deletion ddl/ddl_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,14 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
if col == nil {
return nil, infoschema.ErrColumnNotExists.GenByArgs(originalColName, ident.Name)
}
newColName := specNewColumn.Name.Name
// If we want to rename the column name, we need to check whether it already exists.
if newColName.L != originalColName.L {
Copy link
Member

@jackysp jackysp Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need to check it twice? Both in ddl_api.go and column.go .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is possible to have two identical rename operations in the DDL job list at the same time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the go test coverage will cover the both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

c := table.FindCol(t.Cols(), newColName.L)
if c != nil {
return nil, infoschema.ErrColumnExists.GenByArgs(newColName)
}
}

// Constraints in the new column means adding new constraints. Errors should thrown,
// which will be done by `setDefaultAndComment` later.
Expand All @@ -1362,7 +1370,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
ID: col.ID,
OriginDefaultValue: col.OriginDefaultValue,
FieldType: *specNewColumn.Tp,
Name: specNewColumn.Name.Name,
Name: newColName,
})

err = setCharsetCollationFlenDecimal(&newCol.FieldType)
Expand Down
3 changes: 3 additions & 0 deletions ddl/ddl_db_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,9 @@ func (s *testDBSuite) TestChangeColumn(c *C) {
s.testErrorCode(c, sql, tmysql.ErrUnknown)
sql = "alter table t3 modify en enum('a', 'z', 'b', 'c') not null default 'a'"
s.testErrorCode(c, sql, tmysql.ErrUnknown)
s.mustExec(c, "alter table t3 add column a bigint")
sql = "alter table t3 change aa a bigint"
s.testErrorCode(c, sql, tmysql.ErrDupFieldName)

s.tk.MustExec("drop table t3")
}
Expand Down