-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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.
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
ddl:support alter table comment #6127
Conversation
What is the related github issue? |
model/ddl.go
Outdated
@@ -45,6 +45,7 @@ const ( | |||
ActionRenameTable ActionType = 14 | |||
ActionSetDefaultValue ActionType = 15 | |||
ActionShardRowID ActionType = 16 | |||
ActionSetTableComment ActionType = 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/ActionSetTableComment/ActionModifyTableComment/
ddl/table.go
Outdated
@@ -335,6 +335,30 @@ func (d *ddl) onRenameTable(t *meta.Meta, job *model.Job) (ver int64, _ error) { | |||
return ver, nil | |||
} | |||
|
|||
func (d *ddl) onSetTableComment(t *meta.Meta, job *model.Job) (ver int64, _ error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/onSetTableComment/onModifyTableComment/
ddl/ddl_api.go
Outdated
@@ -1436,6 +1439,31 @@ func (d *ddl) AlterColumn(ctx sessionctx.Context, ident ast.Ident, spec *ast.Alt | |||
return errors.Trace(err) | |||
} | |||
|
|||
func (d *ddl) TableComment(ctx sessionctx.Context, ident ast.Ident, spec *ast.AlterTableSpec) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/TableComment/AlterTableComment/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments before exported method
ddl/ddl_api.go
Outdated
TableID: tb.Meta().ID, | ||
Type: model.ActionSetTableComment, | ||
BinlogInfo: &model.HistoryInfo{}, | ||
Args: []interface{}{schema.ID, spec.Name, spec.Comment}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arg schema.ID
is unnecessary, you can use job.SchemaID
ddl/table.go
Outdated
func (d *ddl) onModifyTableComment(t *meta.Meta, job *model.Job) (ver int64, _ error) { | ||
var tableName string | ||
var comment string | ||
if err := job.DecodeArgs(&tableName, &comment); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tableName is useless?
@zimulala PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ddl/table.go
Outdated
tblInfo.Comment = comment | ||
ver, err = updateVersionAndTableInfo(t, job, tblInfo, true) | ||
if err != nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this blank line.
} | ||
|
||
tb, err := is.TableByName(ident.Schema, ident.Name) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some tests like TestMySQLErrorCode
in ddl_db_test.go
@zimulala PTAL |
/run-integration-common-test tidb-test=pr/481 |
/run-all-tests |
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Added a command support alter table comment respond issues #4990
alter table mytable comment 'this is table comment';
.