ddl: make alter database set tiflash replica skip sequence and view (p…

…ingcap#52348)

close pingcap#51990

pkg/ddl/ddl_api.go

@@ -406,29 +406,24 @@ func (d *ddl) ModifySchemaSetTiFlashReplica(sctx sessionctx.Context, stmt *ast.A
 
 		tbReplicaInfo := tbl.TiFlashReplica
 		if !shouldModifyTiFlashReplica(tbReplicaInfo, tiflashReplica) {
-			logutil.BgLogger().Info("skip repeated processing table", zap.Int64("tableID", tbl.ID), zap.Int64("schemaID", dbInfo.ID), zap.String("tableName", tbl.Name.String()), zap.String("schemaName", dbInfo.Name.String()))
+			logutil.BgLogger().Info("skip repeated processing table",
+				zap.Int64("tableID", tbl.ID),
+				zap.Int64("schemaID", dbInfo.ID),
+				zap.String("tableName", tbl.Name.String()),
+				zap.String("schemaName", dbInfo.Name.String()))
 			skip++
 			continue
 		}
 
-		// Ban setting replica count for tables in system database.
-		if tbl.TempTableType != model.TempTableNone {
-			logutil.BgLogger().Info("skip processing temporary table", zap.Int64("tableID", tbl.ID), zap.Int64("schemaID", dbInfo.ID), zap.String("tableName", tbl.Name.String()), zap.String("schemaName", dbInfo.Name.String()))
-			skip++
-			continue
-		}
-
-		charsetOk := true
-		// Ban setting replica count for tables which has charset not supported by TiFlash
-		for _, col := range tbl.Cols() {
-			_, ok := charset.TiFlashSupportedCharsets[col.GetCharset()]
-			if !ok {
-				charsetOk = false
-				break
-			}
-		}
-		if !charsetOk {
-			logutil.BgLogger().Info("skip processing schema table, unsupported charset", zap.Int64("tableID", tbl.ID), zap.Int64("schemaID", dbInfo.ID), zap.String("tableName", tbl.Name.String()), zap.String("schemaName", dbInfo.Name.String()))
+		// If table is not supported, add err to warnings.
+		err = isTableTiFlashSupported(dbName, tbl)
+		if err != nil {
+			logutil.BgLogger().Info("skip processing table", zap.Int64("tableID", tbl.ID),
+				zap.Int64("schemaID", dbInfo.ID),
+				zap.String("tableName", tbl.Name.String()),
+				zap.String("schemaName", dbInfo.Name.String()),
+				zap.Error(err))
+			sctx.GetSessionVars().StmtCtx.AppendNote(err)
 			skip++
 			continue
 		}
@@ -6517,7 +6512,7 @@ func (d *ddl) AlterTableSetTiFlashReplica(ctx sessionctx.Context, ident ast.Iden
 		return errors.Trace(err)
 	}
 
-	err = isTableTiFlashSupported(schema, tb)
+	err = isTableTiFlashSupported(schema.Name, tb.Meta())
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -6646,16 +6641,18 @@ func (d *ddl) AlterTableRemoveTTL(ctx sessionctx.Context, ident ast.Ident) error
 	return nil
 }
 
-func isTableTiFlashSupported(schema *model.DBInfo, tb table.Table) error {
+func isTableTiFlashSupported(dbName model.CIStr, tbl *model.TableInfo) error {
 	// Memory tables and system tables are not supported by TiFlash
-	if util.IsMemOrSysDB(schema.Name.L) {
+	if util.IsMemOrSysDB(dbName.L) {
 		return errors.Trace(dbterror.ErrUnsupportedTiFlashOperationForSysOrMemTable)
-	} else if tb.Meta().TempTableType != model.TempTableNone {
+	} else if tbl.TempTableType != model.TempTableNone {
 		return dbterror.ErrOptOnTemporaryTable.GenWithStackByArgs("set on tiflash")
+	} else if tbl.IsView() || tbl.IsSequence() {
+		return dbterror.ErrWrongObject.GenWithStackByArgs(dbName, tbl.Name, "BASE TABLE")
 	}
 
 	// Tables that has charset are not supported by TiFlash
-	for _, col := range tb.Cols() {
+	for _, col := range tbl.Cols() {
 		_, ok := charset.TiFlashSupportedCharsets[col.GetCharset()]
 		if !ok {
 			return dbterror.ErrUnsupportedTiFlashOperationForUnsupportedCharsetTable.GenWithStackByArgs(col.GetCharset())

pkg/ddl/tests/tiflash/ddl_tiflash_test.go

@@ -801,6 +801,20 @@ func TestAlterDatabaseBasic(t *testing.T) {
 
 	// There is less TiFlash store
 	tk.MustGetErrMsg("alter database tiflash_ddl set tiflash replica 3", "the tiflash replica count: 3 should be less than the total tiflash server count: 2")
+
+	// Test Issue #51990, alter database skip set tiflash replica on sequence and view.
+	tk.MustExec("create database tiflash_ddl_skip;")
+	tk.MustExec("use tiflash_ddl_skip")
+	tk.MustExec("create table t (id int);")
+	tk.MustExec("create sequence t_seq;")
+	tk.MustExec("create view t_view as select id from t;")
+	tk.MustExec("create global temporary table t_temp (id int) on commit delete rows;")
+	tk.MustExec("alter database tiflash_ddl_skip set tiflash replica 1;")
+	require.Equal(t, "In total 4 tables: 1 succeed, 0 failed, 3 skipped", tk.Session().GetSessionVars().StmtCtx.GetMessage())
+	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."))
 }
 
 func execWithTimeout(t *testing.T, tk *testkit.TestKit, to time.Duration, sql string) (bool, error) {