From 0a2bb9a97d4fc553f9756f3eafa75956cc740b9e Mon Sep 17 00:00:00 2001 From: tangenta Date: Wed, 3 Jul 2019 21:57:14 +0800 Subject: [PATCH 1/5] ddl: refine error messages in unsupported column options --- ddl/db_integration_test.go | 14 ++++++++++++++ ddl/ddl_api.go | 14 ++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 4943c00432b6e..8171950eab140 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -770,6 +770,20 @@ func (s *testIntegrationSuite4) TestChangingTableCharset(c *C) { } +func (s *testIntegrationSuite5) TestChangingColumnCollation(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test") + tk.MustExec("use test") + + tk.MustExec("drop table if exists t1") + tk.MustExec("create table t1 (b char(1) default null) engine=InnoDB default charset=utf8mb4 collate=utf8mb4_general_ci") + tk.MustExec("alter table t1 modify column b char(1) character set utf8mb4 collate utf8mb4_general_ci") + + tk.MustExec("drop table t1") + tk.MustExec("create table t1 (b char(1) collate utf8mb4_general_ci)") + tk.MustExec("alter table t1 modify b char(1) character set utf8mb4 collate utf8mb4_general_ci") +} + func (s *testIntegrationSuite2) TestCaseInsensitiveCharsetAndCollate(c *C) { tk := testkit.NewTestKit(c, s.store) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index a279c501dd0ca..3bf795189a64a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2520,8 +2520,13 @@ func processColumnOptions(ctx sessionctx.Context, col *table.Column, options []* for _, colName := range findColumnNamesInExpr(opt.Expr) { col.Dependences[colName.Name.L] = struct{}{} } + case ast.ColumnOptionCollate: + col.Collate = opt.StrValue + case ast.ColumnOptionReference: + return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("with references")) + case ast.ColumnOptionFulltext: + return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("with full text")) default: - // TODO: Support other types. return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs(opt.Tp)) } } @@ -2604,11 +2609,12 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or if err != nil { return nil, errors.Trace(err) } - err = modifiable(&col.FieldType, &newCol.FieldType) - if err != nil { + + if err = processColumnOptions(ctx, newCol, specNewColumn.Options); err != nil { return nil, errors.Trace(err) } - if err = processColumnOptions(ctx, newCol, specNewColumn.Options); err != nil { + + if err = modifiable(&col.FieldType, &newCol.FieldType); err != nil { return nil, errors.Trace(err) } From 99bac73ac1c29609a2e707418749685a69f9faf9 Mon Sep 17 00:00:00 2001 From: tangenta Date: Thu, 4 Jul 2019 09:46:16 +0800 Subject: [PATCH 2/5] update test --- ddl/db_integration_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 8171950eab140..83aaee63ae400 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -770,18 +770,32 @@ func (s *testIntegrationSuite4) TestChangingTableCharset(c *C) { } -func (s *testIntegrationSuite5) TestChangingColumnCollation(c *C) { +func (s *testIntegrationSuite5) TestModifyingColumnOption(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test") tk.MustExec("use test") + errCodeStr1 := "[ddl:210]" // unsupported modify collate from utf8mb4_bin to utf8mb4_general_ci" + errCodeStr2 := "[ddl:203]" // unsupported modify column with references + assertErrCode := func(sql string, errCodeStr string) { + _, err := tk.Exec(sql) + c.Assert(err, NotNil) + c.Assert(err.Error()[:len(errCodeStr)], Equals, errCodeStr) + } + tk.MustExec("drop table if exists t1") tk.MustExec("create table t1 (b char(1) default null) engine=InnoDB default charset=utf8mb4 collate=utf8mb4_general_ci") - tk.MustExec("alter table t1 modify column b char(1) character set utf8mb4 collate utf8mb4_general_ci") + assertErrCode("alter table t1 modify column b char(1) character set utf8mb4 collate utf8mb4_general_ci", errCodeStr1) tk.MustExec("drop table t1") tk.MustExec("create table t1 (b char(1) collate utf8mb4_general_ci)") tk.MustExec("alter table t1 modify b char(1) character set utf8mb4 collate utf8mb4_general_ci") + + tk.MustExec("drop table t1") + tk.MustExec("drop table if exists t2") + tk.MustExec("create table t1 (a int)") + tk.MustExec("create table t2 (b int, c int)") + assertErrCode("alter table t2 modify column c int references t1(a)", errCodeStr2) } func (s *testIntegrationSuite2) TestCaseInsensitiveCharsetAndCollate(c *C) { From a5818867251684d34e61ad86f56d6d2be839e443 Mon Sep 17 00:00:00 2001 From: tangenta Date: Wed, 10 Jul 2019 15:39:13 +0800 Subject: [PATCH 3/5] refine error message --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 3bf795189a64a..33ed81de0b757 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2527,7 +2527,7 @@ func processColumnOptions(ctx sessionctx.Context, col *table.Column, options []* case ast.ColumnOptionFulltext: return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("with full text")) default: - return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs(opt.Tp)) + return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("unknown column option type")) } } From 73a60a8dca7480d8ebaac52d9a6bf2b107bdb6a5 Mon Sep 17 00:00:00 2001 From: tangenta Date: Wed, 10 Jul 2019 17:59:38 +0800 Subject: [PATCH 4/5] add option type report --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 33ed81de0b757..5a930f6aafcf6 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -2527,7 +2527,7 @@ func processColumnOptions(ctx sessionctx.Context, col *table.Column, options []* case ast.ColumnOptionFulltext: return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("with full text")) default: - return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs("unknown column option type")) + return errors.Trace(errUnsupportedModifyColumn.GenWithStackByArgs(fmt.Sprintf("unknown column option type: %d", opt.Tp))) } } From 96b24b7fc580b7d0458eab8ce441af3d83f64d19 Mon Sep 17 00:00:00 2001 From: tangenta Date: Thu, 11 Jul 2019 15:06:27 +0800 Subject: [PATCH 5/5] update test --- ddl/db_integration_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 83aaee63ae400..367549acc13cb 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -775,8 +775,7 @@ func (s *testIntegrationSuite5) TestModifyingColumnOption(c *C) { tk.MustExec("create database if not exists test") tk.MustExec("use test") - errCodeStr1 := "[ddl:210]" // unsupported modify collate from utf8mb4_bin to utf8mb4_general_ci" - errCodeStr2 := "[ddl:203]" // unsupported modify column with references + errMsg := "[ddl:203]" // unsupported modify column with references assertErrCode := func(sql string, errCodeStr string) { _, err := tk.Exec(sql) c.Assert(err, NotNil) @@ -785,7 +784,7 @@ func (s *testIntegrationSuite5) TestModifyingColumnOption(c *C) { tk.MustExec("drop table if exists t1") tk.MustExec("create table t1 (b char(1) default null) engine=InnoDB default charset=utf8mb4 collate=utf8mb4_general_ci") - assertErrCode("alter table t1 modify column b char(1) character set utf8mb4 collate utf8mb4_general_ci", errCodeStr1) + tk.MustExec("alter table t1 modify column b char(1) character set utf8mb4 collate utf8mb4_general_ci") tk.MustExec("drop table t1") tk.MustExec("create table t1 (b char(1) collate utf8mb4_general_ci)") @@ -795,7 +794,7 @@ func (s *testIntegrationSuite5) TestModifyingColumnOption(c *C) { tk.MustExec("drop table if exists t2") tk.MustExec("create table t1 (a int)") tk.MustExec("create table t2 (b int, c int)") - assertErrCode("alter table t2 modify column c int references t1(a)", errCodeStr2) + assertErrCode("alter table t2 modify column c int references t1(a)", errMsg) } func (s *testIntegrationSuite2) TestCaseInsensitiveCharsetAndCollate(c *C) {