*: Remove variable tidb_enable_alter_placement (#31066)

* *: Remove variable `tidb_enable_alter_placement`
pingcap · Jan 6, 2022 · fac8bf3 · fac8bf3
1 parent 50520e3
commit fac8bf3
Show file tree

Hide file tree

Showing 14 changed files with 6 additions and 70 deletions.
diff --git a/ddl/db_test.go b/ddl/db_test.go
@@ -127,8 +127,6 @@ func setUpSuite(s *testDBSuite, c *C) {
 	c.Assert(err, IsNil)
 	_, err = s.s.Execute(context.Background(), "set @@global.tidb_max_delta_schema_count= 4096")
 	c.Assert(err, IsNil)
-	_, err = s.s.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
-	c.Assert(err, IsNil)
 }
 
 func tearDownSuite(s *testDBSuite, c *C) {

diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go
@@ -236,9 +236,6 @@ func (d *ddl) AlterTablePlacement(ctx sessionctx.Context, ident ast.Ident, place
 }
 
 func checkAndNormalizePlacement(ctx sessionctx.Context, placementPolicyRef *model.PolicyRefInfo, directPlacementOpts *model.PlacementSettings, fallbackPlacementPolicyRef *model.PolicyRefInfo, fallbackDirectPlacementOpts *model.PlacementSettings) (*model.PolicyRefInfo, *model.PlacementSettings, error) {
-	if !ctx.GetSessionVars().EnableAlterPlacement && (placementPolicyRef != nil || directPlacementOpts != nil) {
-		return nil, nil, ErrPlacementDisabled
-	}
 	if placementPolicyRef != nil && directPlacementOpts != nil {
 		return nil, nil, errors.Trace(ErrPlacementPolicyWithDirectOption.GenWithStackByArgs(placementPolicyRef.Name))
 	}

diff --git a/ddl/error.go b/ddl/error.go
@@ -269,9 +269,6 @@ var (
 	// ErrPlacementPolicyInUse is returned when placement policy is in use in drop/alter.
 	ErrPlacementPolicyInUse = dbterror.ClassDDL.NewStd(mysql.ErrPlacementPolicyInUse)
 
-	// ErrPlacementDisabled  is returned when tidb_enable_alter_placement = 0
-	ErrPlacementDisabled = dbterror.ClassDDL.NewStdErr(mysql.ErrUnsupportedDDLOperation, parser_mysql.Message("Alter Placement Rule is disabled, please set 'tidb_enable_alter_placement' if you need to enable it", nil))
-
 	// ErrMultipleDefConstInListPart returns multiple definition of same constant in list partitioning.
 	ErrMultipleDefConstInListPart = dbterror.ClassDDL.NewStd(mysql.ErrMultipleDefConstInListPart)
 

diff --git a/ddl/placement_sql_test.go b/ddl/placement_sql_test.go
@@ -15,7 +15,6 @@
 package ddl_test
 
 import (
-	"context"
 	"fmt"
 	"sort"
 
@@ -34,13 +33,11 @@ import (
 func (s *testDBSuite1) TestPlacementPolicyCache(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	tk.MustExec("use test")
-	tk.Se.GetSessionVars().EnableAlterPlacement = true
 	tk.MustExec("set @@tidb_enable_exchange_partition = 1")
 	defer func() {
 		tk.MustExec("set @@tidb_enable_exchange_partition = 0")
 		tk.MustExec("drop table if exists t1")
 		tk.MustExec("drop table if exists t2")
-		tk.Se.GetSessionVars().EnableAlterPlacement = false
 	}()
 
 	initTable := func() []string {
@@ -103,10 +100,8 @@ func (s *testSerialDBSuite) TestTxnScopeConstraint(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	tk.MustExec("use test")
 	tk.MustExec("drop table if exists t1")
-	tk.Se.GetSessionVars().EnableAlterPlacement = true
 	defer func() {
 		tk.MustExec("drop table if exists t1")
-		tk.Se.GetSessionVars().EnableAlterPlacement = false
 	}()
 
 	tk.MustExec(`create table t1 (c int)
@@ -257,13 +252,11 @@ func (s *testDBSuite6) TestCreateSchemaWithPlacement(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	tk.MustExec("drop schema if exists SchemaDirectPlacementTest")
 	tk.MustExec("drop schema if exists SchemaPolicyPlacementTest")
-	tk.Se.GetSessionVars().EnableAlterPlacement = true
 	defer func() {
 		tk.MustExec("drop schema if exists SchemaDirectPlacementTest")
 		tk.MustExec("drop schema if exists SchemaPolicyPlacementTest")
 		tk.MustExec("drop placement policy if exists PolicySchemaTest")
 		tk.MustExec("drop placement policy if exists PolicyTableTest")
-		tk.Se.GetSessionVars().EnableAlterPlacement = false
 	}()
 
 	tk.MustExec(`CREATE SCHEMA SchemaDirectPlacementTest PRIMARY_REGION='nl' REGIONS = "se,nz,nl" FOLLOWERS=3`)
@@ -445,42 +438,8 @@ func (s *testDBSuite6) TestAlterDBPlacement(c *C) {
 	))
 }
 
-func (s *testDBSuite6) TestEnablePlacementCheck(c *C) {
-
-	tk := testkit.NewTestKit(c, s.store)
-	se, err := session.CreateSession4Test(s.store)
-	c.Assert(err, IsNil)
-	_, err = se.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
-	c.Assert(err, IsNil)
-
-	tk.MustExec("drop database if exists TestPlacementDB;")
-	tk.MustExec("create database TestPlacementDB;")
-	tk.MustExec("use TestPlacementDB;")
-	tk.MustExec("drop placement policy if exists placement_x;")
-	tk.MustExec("create placement policy placement_x PRIMARY_REGION=\"cn-east-1\", REGIONS=\"cn-east-1\";")
-	se.GetSessionVars().EnableAlterPlacement = true
-	tk.MustExec("create table t(c int) partition by range (c) (partition p1 values less than (200) followers=2);")
-	defer func() {
-		tk.MustExec("drop database if exists TestPlacementDB;")
-		tk.MustExec("drop placement policy if exists placement_x;")
-	}()
-
-	tk.Se.GetSessionVars().EnableAlterPlacement = false
-	tk.MustGetErrCode("create database TestPlacementDB2 followers=2;", mysql.ErrUnsupportedDDLOperation)
-	tk.MustGetErrCode("alter database TestPlacementDB placement policy=placement_x", mysql.ErrUnsupportedDDLOperation)
-	tk.MustGetErrCode("create table t (c int) FOLLOWERS=2;", mysql.ErrUnsupportedDDLOperation)
-	tk.MustGetErrCode("alter table t voters=2;", mysql.ErrUnsupportedDDLOperation)
-	tk.MustGetErrCode("create table m (c int) partition by range (c) (partition p1 values less than (200) followers=2);", mysql.ErrUnsupportedDDLOperation)
-	tk.MustGetErrCode("alter table t partition p1 placement policy=\"placement_x\";", mysql.ErrUnsupportedDDLOperation)
-}
-
 func (s *testDBSuite6) TestPlacementTiflashCheck(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
-	se, err := session.CreateSession4Test(s.store)
-	c.Assert(err, IsNil)
-	_, err = se.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
-	c.Assert(err, IsNil)
-
 	c.Assert(failpoint.Enable("github.com/pingcap/tidb/infoschema/mockTiFlashStoreCount", `return(true)`), IsNil)
 	defer func() {
 		err := failpoint.Disable("github.com/pingcap/tidb/infoschema/mockTiFlashStoreCount")
@@ -501,7 +460,7 @@ func (s *testDBSuite6) TestPlacementTiflashCheck(c *C) {
 	defer tk.MustExec("drop table if exists tp")
 	tk.MustExec("alter table tp set tiflash replica 1")
 
-	err = tk.ExecToErr("alter table tp placement policy p1")
+	err := tk.ExecToErr("alter table tp placement policy p1")
 	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)
 	err = tk.ExecToErr("alter table tp primary_region='r2' regions='r2'")
 	c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue)

diff --git a/ddl/serial_test.go b/ddl/serial_test.go
@@ -530,17 +530,13 @@ func (s *testSerialSuite) TestCreateTableWithLike(c *C) {
 
 func (s *testSerialSuite) TestCreateTableWithLikeAtTemporaryMode(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
-	se, err := session.CreateSession4Test(s.store)
-	c.Assert(err, IsNil)
-	_, err = se.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
-	c.Assert(err, IsNil)
 
 	// Test create table like at temporary mode.
 	tk.MustExec("use test")
 	tk.MustExec("drop table if exists temporary_table;")
 	tk.MustExec("create global temporary table temporary_table (a int, b int,index(a)) on commit delete rows")
 	tk.MustExec("drop table if exists temporary_table_t1;")
-	_, err = tk.Exec("create table temporary_table_t1 like temporary_table")
+	_, err := tk.Exec("create table temporary_table_t1 like temporary_table")
 	c.Assert(err.Error(), Equals, core.ErrOptOnTemporaryTable.GenWithStackByArgs("create table like").Error())
 	tk.MustExec("drop table if exists temporary_table;")
 

diff --git a/docs/design/2021-07-29-hidden-sysvars.md b/docs/design/2021-07-29-hidden-sysvars.md
@@ -39,7 +39,6 @@ There are currently ~20 hidden system variables:
 | tidb_enable_pipelined_window_function | ‘Pipelined window function’ feature | #1 Stable |
 | tidb_enable_change_multi_schema | ‘Change multi schema in one statement’ feature. | #3 In Development Feature |
 | tidb_enable_point_get_cache | ‘point get cache’ feature | #2 Experimental |
-| tidb_enable_alter_placement | placement rules in SQL feature | #3 In Development Feature |
 | tidb_enable_extended_stats | ‘extended stats’ feature | #2 Experimental |
 | tidb_partition_prune_mode | Is partition prune mode dynamic or static | #2 Experimental |
 | tidb_enable_async_commit | Support Async Commit PRD  | #1 Stable |

diff --git a/executor/executor_test.go b/executor/executor_test.go
@@ -210,8 +210,6 @@ func (s *baseTestSuite) SetUpSuite(c *C) {
 	c.Assert(err, IsNil)
 	se, err := session.CreateSession4Test(s.store)
 	c.Assert(err, IsNil)
-	_, err = se.Execute(context.Background(), "set @@global.tidb_enable_alter_placement=1")
-	c.Assert(err, IsNil)
 	se.Close()
 	d.SetStatsUpdating(true)
 	s.domain = d

diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go
@@ -2595,7 +2595,6 @@ func TestInformationSchemaPlacmentRulesPrivileges(t *testing.T) {
 	}()
 	tk.MustExec("CREATE DATABASE placement_rule_db")
 	tk.MustExec("USE placement_rule_db")
-	tk.Session().GetSessionVars().EnableAlterPlacement = true
 	tk.MustExec(`CREATE TABLE placement_rule_table_se (a int) PRIMARY_REGION="se" REGIONS="se,nl"`)
 	tk.MustExec(`CREATE TABLE placement_rule_table_nl (a int) PRIMARY_REGION="nl" REGIONS="se,nl"`)
 	tk.MustQuery(`SELECT * FROM information_schema.placement_rules WHERE SCHEMA_NAME = "placement_rule_db"`).Sort().Check(testkit.Rows(

diff --git a/sessionctx/variable/removed.go b/sessionctx/variable/removed.go
@@ -22,6 +22,7 @@ package variable
 // careful not to return dummy data.
 
 var removedSysVars = map[string]string{
+	TiDBEnableAlterPlacement:       "alter placement is now always enabled",
 	TiDBEnableGlobalTemporaryTable: "temporary table support is now always enabled",
 	TiDBSlowLogMasking:             "use tidb_redact_log instead",
 }

diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go
@@ -743,9 +743,6 @@ type SessionVars struct {
 	// EnablePointGetCache is used to cache value for point get for read only scenario.
 	EnablePointGetCache bool
 
-	// EnableAlterPlacement indicates whether a user can alter table partition placement rules.
-	EnableAlterPlacement bool
-
 	// EnablePlacementChecks indicates whether a user can check validation of placement.
 	EnablePlacementChecks bool
 
@@ -1227,7 +1224,6 @@ func NewSessionVars() *SessionVars {
 		ShardAllocateStep:           DefTiDBShardAllocateStep,
 		EnableChangeMultiSchema:     DefTiDBChangeMultiSchema,
 		EnablePointGetCache:         DefTiDBPointGetCache,
-		EnableAlterPlacement:        DefTiDBEnableAlterPlacement,
 		EnableAmendPessimisticTxn:   DefTiDBEnableAmendPessimisticTxn,
 		PartitionPruneMode:          *atomic2.NewString(DefTiDBPartitionPruneMode),
 		TxnScope:                    kv.NewDefaultTxnScopeVar(),

diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go
@@ -853,10 +853,6 @@ var defaultSysVars = []*SysVar{
 		s.EnablePointGetCache = TiDBOptOn(val)
 		return nil
 	}},
-	{Scope: ScopeGlobal, Name: TiDBEnableAlterPlacement, Value: BoolToOnOff(DefTiDBEnableAlterPlacement), Hidden: true, Type: TypeBool, SetSession: func(s *SessionVars, val string) error {
-		s.EnableAlterPlacement = TiDBOptOn(val)
-		return nil
-	}},
 	{Scope: ScopeSession, Name: TiDBForcePriority, skipInit: true, Value: mysql.Priority2Str[DefTiDBForcePriority], SetSession: func(s *SessionVars, val string) error {
 		atomic.StoreInt32(&ForcePriority, int32(mysql.Str2Priority(val)))
 		return nil

diff --git a/sessionctx/variable/sysvar_test.go b/sessionctx/variable/sysvar_test.go
@@ -668,7 +668,7 @@ func TestSettersandGetters(t *testing.T) {
 			// There are some historial exceptions where global variables are loaded into the session.
 			// Please don't add to this list, the behavior is not MySQL compatible.
 			switch sv.Name {
-			case TiDBEnableChangeMultiSchema, TiDBDDLReorgBatchSize, TiDBEnableAlterPlacement,
+			case TiDBEnableChangeMultiSchema, TiDBDDLReorgBatchSize,
 				TiDBMaxDeltaSchemaCount, InitConnect, MaxPreparedStmtCount,
 				TiDBDDLReorgWorkerCount, TiDBDDLErrorCountLimit, TiDBRowFormatVersion,
 				TiDBEnableTelemetry, TiDBEnablePointGetCache:

diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go
@@ -414,6 +414,7 @@ const (
 	TiDBEnablePointGetCache = "tidb_enable_point_get_cache"
 
 	// TiDBEnableAlterPlacement is used to control whether to enable alter table partition.
+	// Deprecated: It is removed and do not use it again
 	TiDBEnableAlterPlacement = "tidb_enable_alter_placement"
 
 	// tidb_max_delta_schema_count defines the max length of deltaSchemaInfos.
@@ -732,7 +733,6 @@ const (
 	DefTiDBMaxDeltaSchemaCount            = 1024
 	DefTiDBChangeMultiSchema              = false
 	DefTiDBPointGetCache                  = false
-	DefTiDBEnableAlterPlacement           = false
 	DefTiDBEnableAutoIncrementInGenerated = false
 	DefTiDBHashAggPartialConcurrency      = ConcurrencyUnset
 	DefTiDBHashAggFinalConcurrency        = ConcurrencyUnset

diff --git a/sessionctx/variable/variable.go b/sessionctx/variable/variable.go
@@ -497,7 +497,7 @@ func (sv *SysVar) SkipInit() bool {
 	// These a special "Global-only" sysvars that for backward compatibility
 	// are currently cached in the session. Please don't add to this list.
 	switch sv.Name {
-	case TiDBEnableChangeMultiSchema, TiDBDDLReorgBatchSize, TiDBEnableAlterPlacement,
+	case TiDBEnableChangeMultiSchema, TiDBDDLReorgBatchSize,
 		TiDBMaxDeltaSchemaCount, InitConnect, MaxPreparedStmtCount,
 		TiDBDDLReorgWorkerCount, TiDBDDLErrorCountLimit, TiDBRowFormatVersion,
 		TiDBEnableTelemetry, TiDBEnablePointGetCache: