From 382a744371839c5c8bad64eac365cc31b74a36e3 Mon Sep 17 00:00:00 2001 From: Lynn Date: Fri, 11 Oct 2024 17:44:23 +0800 Subject: [PATCH 1/2] ddl: fix a but that create table with vector index and gbk charset column --- pkg/ddl/create_table.go | 11 +++++++---- pkg/ddl/executor.go | 2 +- pkg/ddl/index_modify_test.go | 15 +++++++++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/pkg/ddl/create_table.go b/pkg/ddl/create_table.go index 5aba1fde1f369..95279448bd90f 100644 --- a/pkg/ddl/create_table.go +++ b/pkg/ddl/create_table.go @@ -410,7 +410,7 @@ func buildTableInfoWithCheck(ctx *metabuild.Context, store kv.Storage, s *ast.Cr if err = checkTableInfoValidWithStmt(ctx, tbInfo, s); err != nil { return nil, err } - if err = checkTableInfoValidExtra(ctx.GetExprCtx().GetEvalCtx().ErrCtx(), store, tbInfo); err != nil { + if err = checkTableInfoValidExtra(ctx.GetExprCtx().GetEvalCtx().ErrCtx(), store, s.Table.Schema, tbInfo); err != nil { return nil, err } return tbInfo, nil @@ -511,7 +511,7 @@ func checkGeneratedColumn(ctx *metabuild.Context, schemaName pmodel.CIStr, table return nil } -func checkVectorIndexIfNeedTiFlashReplica(store kv.Storage, tblInfo *model.TableInfo) error { +func checkVectorIndexIfNeedTiFlashReplica(store kv.Storage, dbName pmodel.CIStr, tblInfo *model.TableInfo) error { var hasVectorIndex bool for _, idx := range tblInfo.Indices { if idx.VectorInfo != nil { @@ -525,6 +525,9 @@ func checkVectorIndexIfNeedTiFlashReplica(store kv.Storage, tblInfo *model.Table if store == nil { return errors.New("the store is nil") } + if err := isTableTiFlashSupported(dbName, tblInfo); err != nil { + return errors.Trace(err) + } if tblInfo.TiFlashReplica == nil || tblInfo.TiFlashReplica.Count == 0 { replicas, err := infoschema.GetTiFlashStoreCount(store) @@ -551,7 +554,7 @@ func checkVectorIndexIfNeedTiFlashReplica(store kv.Storage, tblInfo *model.Table // name length and column count. // (checkTableInfoValid is also used in repairing objects which don't perform // these checks. Perhaps the two functions should be merged together regardless?) -func checkTableInfoValidExtra(ec errctx.Context, store kv.Storage, tbInfo *model.TableInfo) error { +func checkTableInfoValidExtra(ec errctx.Context, store kv.Storage, dbName pmodel.CIStr, tbInfo *model.TableInfo) error { if err := checkTooLongTable(tbInfo.Name); err != nil { return err } @@ -574,7 +577,7 @@ func checkTableInfoValidExtra(ec errctx.Context, store kv.Storage, tbInfo *model if err := checkGlobalIndexes(ec, tbInfo); err != nil { return errors.Trace(err) } - if err := checkVectorIndexIfNeedTiFlashReplica(store, tbInfo); err != nil { + if err := checkVectorIndexIfNeedTiFlashReplica(store, dbName, tbInfo); err != nil { return errors.Trace(err) } diff --git a/pkg/ddl/executor.go b/pkg/ddl/executor.go index d0a12e9c62564..3aab9acce7cb5 100644 --- a/pkg/ddl/executor.go +++ b/pkg/ddl/executor.go @@ -1089,7 +1089,7 @@ func (e *executor) createTableWithInfoJob( } } - if err := checkTableInfoValidExtra(ctx.GetSessionVars().StmtCtx.ErrCtx(), ctx.GetStore(), tbInfo); err != nil { + if err := checkTableInfoValidExtra(ctx.GetSessionVars().StmtCtx.ErrCtx(), ctx.GetStore(), dbName, tbInfo); err != nil { return nil, err } diff --git a/pkg/ddl/index_modify_test.go b/pkg/ddl/index_modify_test.go index 727998f90cc52..666b56c81124f 100644 --- a/pkg/ddl/index_modify_test.go +++ b/pkg/ddl/index_modify_test.go @@ -1126,6 +1126,10 @@ func TestCreateTableWithVectorIndex(t *testing.T) { require.Equal(t, 1, len(indexes)) require.Equal(t, pmodel.IndexTypeHNSW, indexes[0].Tp) require.Equal(t, model.DistanceMetricCosine, indexes[0].VectorInfo.DistanceMetric) + tk.MustExec("insert into t values (1, '[1,2.1,3.3]');") + tk.MustQuery("select * from t;").Check(testkit.Rows("1 [1,2.1,3.3]")) + tk.MustExec("create view v as select * from tt;") + tk.MustQuery("select * from v;").Check(testkit.Rows("1 [1,2.1,3.3]")) tk.MustExec(`DROP TABLE t`) } @@ -1145,13 +1149,20 @@ func TestCreateTableWithVectorIndex(t *testing.T) { // test unsupported table types tk.MustContainErrMsg("create temporary table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW)", - "`vector index` is unsupported on temporary tables.") + "`set on tiflash` is unsupported on temporary tables.") // global and local temporary table using different way to handle, so we have two test cases. tk.MustContainErrMsg("create global temporary table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW) on commit delete rows;", - "`vector index` is unsupported on temporary tables.") + "`set on tiflash` is unsupported on temporary tables.") tk.MustContainErrMsg("create table pt(id bigint, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW) "+ "partition by range(id) (partition p0 values less than (20), partition p1 values less than (100));", "Unsupported add vector index: unsupported partition table") + tk.MustContainErrMsg("create table t(a int, b vector(3), c char(210) CHARACTER SET gbk COLLATE gbk_bin, vector index((VEC_COSINE_DISTANCE(b))));", + "Unsupported ALTER TiFlash settings for tables not supported by TiFlash: table contains gbk charset") + tk.MustContainErrMsg("create table mysql.t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))));", + "Unsupported ALTER TiFlash settings for system table and memory table") + tk.MustContainErrMsg("create table information_schema.t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))));", + "Unsupported ALTER TiFlash settings for system table and memory table") + // a vector index with invisible tk.MustContainErrMsg("create table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW INVISIBLE)", "Unsupported set vector index invisible") From ac559e581b56e392b161bd13503668dc49478ec1 Mon Sep 17 00:00:00 2001 From: Lynn Date: Sat, 12 Oct 2024 10:51:36 +0800 Subject: [PATCH 2/2] *: update error msg --- pkg/ddl/executor.go | 2 +- pkg/ddl/index_modify_test.go | 10 +++++----- pkg/ddl/tests/tiflash/ddl_tiflash_test.go | 2 +- pkg/ddl/tiflash_replica_test.go | 2 +- pkg/executor/test/tiflashtest/tiflash_test.go | 2 +- pkg/util/dbterror/ddl_terror.go | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/ddl/executor.go b/pkg/ddl/executor.go index 4601ca0af5fab..a7e64dad34614 100644 --- a/pkg/ddl/executor.go +++ b/pkg/ddl/executor.go @@ -3795,7 +3795,7 @@ func isTableTiFlashSupported(dbName pmodel.CIStr, tbl *model.TableInfo) error { if util.IsMemOrSysDB(dbName.L) { return errors.Trace(dbterror.ErrUnsupportedTiFlashOperationForSysOrMemTable) } else if tbl.TempTableType != model.TempTableNone { - return dbterror.ErrOptOnTemporaryTable.GenWithStackByArgs("set on tiflash") + return dbterror.ErrOptOnTemporaryTable.GenWithStackByArgs("set TiFlash replica") } else if tbl.IsView() || tbl.IsSequence() { return dbterror.ErrWrongObject.GenWithStackByArgs(dbName, tbl.Name, "BASE TABLE") } diff --git a/pkg/ddl/index_modify_test.go b/pkg/ddl/index_modify_test.go index 6e078c3d295af..b38d05fbffbd5 100644 --- a/pkg/ddl/index_modify_test.go +++ b/pkg/ddl/index_modify_test.go @@ -1128,19 +1128,19 @@ func TestCreateTableWithVectorIndex(t *testing.T) { // test unsupported table types tk.MustContainErrMsg("create temporary table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW)", - "`set on tiflash` is unsupported on temporary tables.") + "`set TiFlash replica` is unsupported on temporary tables.") // global and local temporary table using different way to handle, so we have two test cases. tk.MustContainErrMsg("create global temporary table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW) on commit delete rows;", - "`set on tiflash` is unsupported on temporary tables.") + "`set TiFlash replica` is unsupported on temporary tables.") tk.MustContainErrMsg("create table pt(id bigint, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW) "+ "partition by range(id) (partition p0 values less than (20), partition p1 values less than (100));", "Unsupported add vector index: unsupported partition table") tk.MustContainErrMsg("create table t(a int, b vector(3), c char(210) CHARACTER SET gbk COLLATE gbk_bin, vector index((VEC_COSINE_DISTANCE(b))));", - "Unsupported ALTER TiFlash settings for tables not supported by TiFlash: table contains gbk charset") + "Unsupported `set TiFlash replica` settings for table contains gbk charset") tk.MustContainErrMsg("create table mysql.t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))));", - "Unsupported ALTER TiFlash settings for system table and memory table") + "Unsupported `set TiFlash replica` settings for system table and memory table") tk.MustContainErrMsg("create table information_schema.t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))));", - "Unsupported ALTER TiFlash settings for system table and memory table") + "Unsupported `set TiFlash replica` settings for system table and memory table") // a vector index with invisible tk.MustContainErrMsg("create table t(a int, b vector(3), vector index((VEC_COSINE_DISTANCE(b))) USING HNSW INVISIBLE)", diff --git a/pkg/ddl/tests/tiflash/ddl_tiflash_test.go b/pkg/ddl/tests/tiflash/ddl_tiflash_test.go index 65cceed97fe5c..32fdebd9dc124 100644 --- a/pkg/ddl/tests/tiflash/ddl_tiflash_test.go +++ b/pkg/ddl/tests/tiflash/ddl_tiflash_test.go @@ -816,7 +816,7 @@ func TestAlterDatabaseBasic(t *testing.T) { tk.MustQuery(`show warnings;`).Sort().Check(testkit.Rows( "Note 1347 'tiflash_ddl_skip.t_seq' is not BASE TABLE", "Note 1347 'tiflash_ddl_skip.t_view' is not BASE TABLE", - "Note 8006 `set on tiflash` is unsupported on temporary tables.")) + "Note 8006 `set TiFlash replica` is unsupported on temporary tables.")) } func execWithTimeout(t *testing.T, tk *testkit.TestKit, to time.Duration, sql string) (bool, error) { diff --git a/pkg/ddl/tiflash_replica_test.go b/pkg/ddl/tiflash_replica_test.go index e3368ed2cccfc..316ad102d9b2b 100644 --- a/pkg/ddl/tiflash_replica_test.go +++ b/pkg/ddl/tiflash_replica_test.go @@ -246,7 +246,7 @@ func TestSetTableFlashReplicaForSystemTable(t *testing.T) { if tbl.Meta().View != nil { require.ErrorIs(t, err, dbterror.ErrWrongObject) } else { - require.Equal(t, "[ddl:8200]Unsupported ALTER TiFlash settings for system table and memory table", err.Error()) + require.Equal(t, "[ddl:8200]Unsupported `set TiFlash replica` settings for system table and memory table", err.Error()) } } else { require.Equal(t, fmt.Sprintf("[planner:1142]ALTER command denied to user 'root'@'%%' for table '%s'", strings.ToLower(one)), err.Error()) diff --git a/pkg/executor/test/tiflashtest/tiflash_test.go b/pkg/executor/test/tiflashtest/tiflash_test.go index e210f120e6a70..03e6e900ee8bf 100644 --- a/pkg/executor/test/tiflashtest/tiflash_test.go +++ b/pkg/executor/test/tiflashtest/tiflash_test.go @@ -71,7 +71,7 @@ func TestNonsupportCharsetTable(t *testing.T) { 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") require.Error(t, err) - require.Equal(t, "[ddl:8200]Unsupported ALTER TiFlash settings for tables not supported by TiFlash: table contains gbk charset", err.Error()) + require.Equal(t, "[ddl:8200]Unsupported `set TiFlash replica` settings for table contains gbk charset", err.Error()) tk.MustExec("drop table if exists t") tk.MustExec("create table t(a char(10) charset utf8)") diff --git a/pkg/util/dbterror/ddl_terror.go b/pkg/util/dbterror/ddl_terror.go index c75a409507177..81c8203c5c10b 100644 --- a/pkg/util/dbterror/ddl_terror.go +++ b/pkg/util/dbterror/ddl_terror.go @@ -419,9 +419,9 @@ var ( // ErrAlterTiFlashModeForTableWithoutTiFlashReplica returns when set tiflash mode on table whose tiflash_replica is null or tiflash_replica_count = 0 ErrAlterTiFlashModeForTableWithoutTiFlashReplica = ClassDDL.NewStdErr(0, parser_mysql.Message("TiFlash mode will take effect after at least one TiFlash replica is set for the table", nil)) // ErrUnsupportedTiFlashOperationForSysOrMemTable means we don't support the alter tiflash related action(e.g. set tiflash mode, set tiflash replica) for system table. - ErrUnsupportedTiFlashOperationForSysOrMemTable = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "ALTER TiFlash settings for system table and memory table"), nil)) + ErrUnsupportedTiFlashOperationForSysOrMemTable = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "`set TiFlash replica` settings for system table and memory table"), nil)) // ErrUnsupportedTiFlashOperationForUnsupportedCharsetTable is used when alter alter tiflash related action(e.g. set tiflash mode, set tiflash replica) with unsupported charset. - ErrUnsupportedTiFlashOperationForUnsupportedCharsetTable = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "ALTER TiFlash settings for tables not supported by TiFlash: table contains %s charset"), nil)) + ErrUnsupportedTiFlashOperationForUnsupportedCharsetTable = ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message(fmt.Sprintf(mysql.MySQLErrName[mysql.ErrUnsupportedDDLOperation].Raw, "`set TiFlash replica` settings for table contains %s charset"), nil)) // ErrTiFlashBackfillIndex is the error that tiflash backfill the index failed. ErrTiFlashBackfillIndex = ClassDDL.NewStdErr(mysql.ErrTiFlashBackfillIndex, parser_mysql.Message(mysql.MySQLErrName[mysql.ErrTiFlashBackfillIndex].Raw, nil))