From c1a430aeb76d6588f475012072e028da5f963a3d Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Thu, 25 Nov 2021 18:30:06 +0800 Subject: [PATCH 1/5] *: forbid set TiFlash Replica for a table with unsupport charset --- ddl/ddl_api.go | 8 ++++++++ ddl/error.go | 2 ++ executor/tiflash_test.go | 18 ++++++++++++++++++ parser/charset/charset.go | 9 +++++++++ 4 files changed, 37 insertions(+) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index fd12090bc8b19..0ba46684c8cab 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -4721,6 +4721,14 @@ func (d *ddl) AlterTableSetTiFlashReplica(ctx sessionctx.Context, ident ast.Iden return ErrOptOnTemporaryTable.GenWithStackByArgs("set tiflash replica") } + // Ban setting replica count for tables which has charset not supported by TiFlash + for _, col := range tb.Cols() { + _, v := charset.TiFlashSupportedCharsets[col.Charset] + if !v { + return ErrAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs(col.Charset) + } + } + tbReplicaInfo := tb.Meta().TiFlashReplica if tbReplicaInfo != nil && tbReplicaInfo.Count == replicaInfo.Count && len(tbReplicaInfo.LocationLabels) == len(replicaInfo.Labels) { diff --git a/ddl/error.go b/ddl/error.go index b3eeb9655fc77..9be0a6161dd4f 100644 --- a/ddl/error.go +++ b/ddl/error.go @@ -65,6 +65,8 @@ var ( errFkColumnCannotDrop = dbterror.ClassDDL.NewStd(mysql.ErrFkColumnCannotDrop) errFKIncompatibleColumns = dbterror.ClassDDL.NewStd(mysql.ErrFKIncompatibleColumns) + ErrAlterReplicaForUnsupportedCharsetTable = dbterror.ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "ALTER table replica for table contain %s charset"), nil)) + errOnlyOnRangeListPartition = dbterror.ClassDDL.NewStd(mysql.ErrOnlyOnRangeListPartition) // errWrongKeyColumn is for table column cannot be indexed. errWrongKeyColumn = dbterror.ClassDDL.NewStd(mysql.ErrWrongKeyColumn) diff --git a/executor/tiflash_test.go b/executor/tiflash_test.go index 5bc9503cc29ff..c8d403d338f1c 100644 --- a/executor/tiflash_test.go +++ b/executor/tiflash_test.go @@ -17,6 +17,7 @@ package executor_test import ( "bytes" "fmt" + "github.com/pingcap/tidb/util/collate" "math" "math/rand" "strings" @@ -28,6 +29,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/kv" @@ -84,6 +86,22 @@ func (s *tiflashTestSuite) TearDownSuite(c *C) { c.Assert(s.store.Close(), IsNil) } +func (s *tiflashTestSuite) TestNonsupportCharsetTable(c *C) { + collate.SetCharsetFeatEnabledForTest(true) + defer collate.SetCharsetFeatEnabledForTest(false) + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a char(10) charset gbk collate gbk_bin)") + err := tk.ExecToErr("alter table t set tiflash replica 1") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, ddl.ErrAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs("gbk").Error()) + + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a char(10) charset utf8)") + tk.MustExec("alter table t set tiflash replica 1") +} + func (s *tiflashTestSuite) TestReadPartitionTable(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") diff --git a/parser/charset/charset.go b/parser/charset/charset.go index e47a65d4c89df..a4344310b91af 100644 --- a/parser/charset/charset.go +++ b/parser/charset/charset.go @@ -68,6 +68,15 @@ var supportedCollationNames = map[string]struct{}{ CollationBin: {}, } +// TiFlashSupportedCharsets is a map which contains TiFlash supports charsets. +var TiFlashSupportedCharsets = map[string]bool{ + CharsetUTF8: true, + CharsetUTF8MB4: true, + CharsetASCII: true, + CharsetLatin1: true, + CharsetBin: true, +} + // GetSupportedCharsets gets descriptions for all charsets supported so far. func GetSupportedCharsets() []*Charset { charsets := make([]*Charset, 0, len(charsetInfos)) From f31c6eb7881796caf47d8c5296d7ef9443d1d6ce Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 26 Nov 2021 09:58:47 +0800 Subject: [PATCH 2/5] fix make fmt --- ddl/ddl_api.go | 2 +- ddl/error.go | 2 +- executor/tiflash_test.go | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 0ba46684c8cab..f427b8fac05c1 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -4725,7 +4725,7 @@ func (d *ddl) AlterTableSetTiFlashReplica(ctx sessionctx.Context, ident ast.Iden for _, col := range tb.Cols() { _, v := charset.TiFlashSupportedCharsets[col.Charset] if !v { - return ErrAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs(col.Charset) + return errAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs(col.Charset) } } diff --git a/ddl/error.go b/ddl/error.go index 9be0a6161dd4f..f95b5baf0e4a9 100644 --- a/ddl/error.go +++ b/ddl/error.go @@ -65,7 +65,7 @@ var ( errFkColumnCannotDrop = dbterror.ClassDDL.NewStd(mysql.ErrFkColumnCannotDrop) errFKIncompatibleColumns = dbterror.ClassDDL.NewStd(mysql.ErrFKIncompatibleColumns) - ErrAlterReplicaForUnsupportedCharsetTable = dbterror.ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "ALTER table replica for table contain %s charset"), nil)) + errAlterReplicaForUnsupportedCharsetTable = dbterror.ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "ALTER table replica for table contain %s charset"), nil)) errOnlyOnRangeListPartition = dbterror.ClassDDL.NewStd(mysql.ErrOnlyOnRangeListPartition) // errWrongKeyColumn is for table column cannot be indexed. diff --git a/executor/tiflash_test.go b/executor/tiflash_test.go index c8d403d338f1c..b6e6d3f60efc5 100644 --- a/executor/tiflash_test.go +++ b/executor/tiflash_test.go @@ -29,7 +29,6 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" - "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/kv" @@ -92,10 +91,10 @@ func (s *tiflashTestSuite) TestNonsupportCharsetTable(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("drop table if exists t") - tk.MustExec("create table t(a char(10) charset gbk collate gbk_bin)") + tk.MustExec("create table t(a int, b char(10) charset gbk collate gbk_bin)") err := tk.ExecToErr("alter table t set tiflash replica 1") c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, ddl.ErrAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs("gbk").Error()) + c.Assert(err.Error(), Equals, "[ddl:8200]Unsupported ALTER table replica for table contain gbk charset") tk.MustExec("drop table if exists t") tk.MustExec("create table t(a char(10) charset utf8)") From cab20f760d58ff72f0783fa023b73d6b2fcba25f Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 26 Nov 2021 10:10:43 +0800 Subject: [PATCH 3/5] fix go fmt --- executor/tiflash_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/executor/tiflash_test.go b/executor/tiflash_test.go index b6e6d3f60efc5..71cf414403752 100644 --- a/executor/tiflash_test.go +++ b/executor/tiflash_test.go @@ -17,7 +17,6 @@ package executor_test import ( "bytes" "fmt" - "github.com/pingcap/tidb/util/collate" "math" "math/rand" "strings" @@ -25,6 +24,8 @@ import ( "sync/atomic" "time" + "github.com/pingcap/tidb/util/collate" + . "github.com/pingcap/check" "github.com/pingcap/errors" "github.com/pingcap/failpoint" From 37bd8049bfa69f43bda11086a31ad80ba5128dca Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 26 Nov 2021 14:16:53 +0800 Subject: [PATCH 4/5] follow comments --- ddl/ddl_api.go | 4 ++-- parser/charset/charset.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index f427b8fac05c1..dab631c06e981 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -4723,8 +4723,8 @@ func (d *ddl) AlterTableSetTiFlashReplica(ctx sessionctx.Context, ident ast.Iden // Ban setting replica count for tables which has charset not supported by TiFlash for _, col := range tb.Cols() { - _, v := charset.TiFlashSupportedCharsets[col.Charset] - if !v { + _, ok := charset.TiFlashSupportedCharsets[col.Charset] + if !ok { return errAlterReplicaForUnsupportedCharsetTable.GenWithStackByArgs(col.Charset) } } diff --git a/parser/charset/charset.go b/parser/charset/charset.go index a4344310b91af..091800dd63003 100644 --- a/parser/charset/charset.go +++ b/parser/charset/charset.go @@ -69,12 +69,12 @@ var supportedCollationNames = map[string]struct{}{ } // TiFlashSupportedCharsets is a map which contains TiFlash supports charsets. -var TiFlashSupportedCharsets = map[string]bool{ - CharsetUTF8: true, - CharsetUTF8MB4: true, - CharsetASCII: true, - CharsetLatin1: true, - CharsetBin: true, +var TiFlashSupportedCharsets = map[string]struct{}{ + CharsetUTF8: {}, + CharsetUTF8MB4: {}, + CharsetASCII: {}, + CharsetLatin1: {}, + CharsetBin: {}, } // GetSupportedCharsets gets descriptions for all charsets supported so far. From 379a0db0a4c0a6ae9f682a9730014896459a75ff Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 26 Nov 2021 14:43:08 +0800 Subject: [PATCH 5/5] follow comments --- executor/tiflash_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/executor/tiflash_test.go b/executor/tiflash_test.go index 71cf414403752..b394d36c6da5b 100644 --- a/executor/tiflash_test.go +++ b/executor/tiflash_test.go @@ -24,8 +24,6 @@ import ( "sync/atomic" "time" - "github.com/pingcap/tidb/util/collate" - . "github.com/pingcap/check" "github.com/pingcap/errors" "github.com/pingcap/failpoint" @@ -39,6 +37,7 @@ import ( "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/mockstore/unistore" + "github.com/pingcap/tidb/util/collate" "github.com/pingcap/tidb/util/israce" "github.com/pingcap/tidb/util/kvcache" "github.com/pingcap/tidb/util/testkit"