From 7391dc78456cf68b825fb704d4703790f809ee8d Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Mon, 17 Jan 2022 15:08:02 +0800 Subject: [PATCH 1/4] *: Forbid add a partition with placement on a table which has tiflash replicas --- ddl/partition.go | 4 ++++ ddl/placement_sql_test.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/ddl/partition.go b/ddl/partition.go index a2d4c84333574..2961886ee353b 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -94,6 +94,10 @@ func (w *worker) onAddTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) (v return ver, err } + if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { + return 0, errors.Trace(ErrIncompatibleTiFlashAndPlacement) + } + // In order to skip maintaining the state check in partitionDefinition, TiDB use addingDefinition instead of state field. // So here using `job.SchemaState` to judge what the stage of this job is. switch job.SchemaState { diff --git a/ddl/placement_sql_test.go b/ddl/placement_sql_test.go index c1febbe18218b..f3ea0231178ee 100644 --- a/ddl/placement_sql_test.go +++ b/ddl/placement_sql_test.go @@ -739,6 +739,8 @@ func (s *testDBSuite6) TestPlacementTiflashCheck(c *C) { c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue) err = tk.ExecToErr("alter table tp partition p0 primary_region='r2' regions='r2'") c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue) + err = tk.ExecToErr("alter table tp add partition(partition p2 VALUES LESS THAN (10000) placement policy p1)") + c.Assert(ddl.ErrIncompatibleTiFlashAndPlacement.Equal(err), IsTrue) tk.MustQuery("show create table tp").Check(testkit.Rows("" + "tp CREATE TABLE `tp` (\n" + " `id` int(11) DEFAULT NULL\n" + From 6b27d29fda5f04f0fb0554deff021e576a1f4d6a Mon Sep 17 00:00:00 2001 From: xhe Date: Mon, 17 Jan 2022 15:41:18 +0800 Subject: [PATCH 2/4] *: refine --- ddl/partition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/partition.go b/ddl/partition.go index 2961886ee353b..499b6c45e643f 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -95,7 +95,7 @@ func (w *worker) onAddTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) (v } if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { - return 0, errors.Trace(ErrIncompatibleTiFlashAndPlacement) + return , errors.Trace(ErrIncompatibleTiFlashAndPlacement) } // In order to skip maintaining the state check in partitionDefinition, TiDB use addingDefinition instead of state field. From 61ec68bfa613aa43938ee8aceec1a2301111b45f Mon Sep 17 00:00:00 2001 From: xhe Date: Mon, 17 Jan 2022 15:42:03 +0800 Subject: [PATCH 3/4] *: mistake --- ddl/partition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/partition.go b/ddl/partition.go index 499b6c45e643f..5459428ee78a3 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -95,7 +95,7 @@ func (w *worker) onAddTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) (v } if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { - return , errors.Trace(ErrIncompatibleTiFlashAndPlacement) + return ver, errors.Trace(ErrIncompatibleTiFlashAndPlacement) } // In order to skip maintaining the state check in partitionDefinition, TiDB use addingDefinition instead of state field. From e291a340f85ef6ff4430fb21f4e3cd0bdeeb9e4b Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Mon, 17 Jan 2022 18:37:40 +0800 Subject: [PATCH 4/4] fix break --- ddl/partition.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ddl/partition.go b/ddl/partition.go index 5459428ee78a3..951c11b8d60ff 100644 --- a/ddl/partition.go +++ b/ddl/partition.go @@ -94,10 +94,6 @@ func (w *worker) onAddTablePartition(d *ddlCtx, t *meta.Meta, job *model.Job) (v return ver, err } - if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 { - return ver, errors.Trace(ErrIncompatibleTiFlashAndPlacement) - } - // In order to skip maintaining the state check in partitionDefinition, TiDB use addingDefinition instead of state field. // So here using `job.SchemaState` to judge what the stage of this job is. switch job.SchemaState { @@ -198,6 +194,10 @@ func alterTablePartitionBundles(t *meta.Meta, tblInfo *model.TableInfo, addingDe p.Definitions = append(tblInfo.Partition.Definitions, addingDefinitions...) tblInfo.Partition = &p + if tblInfo.TiFlashReplica != nil && tblInfo.TiFlashReplica.Count > 0 && tableHasPlacementSettings(tblInfo) { + return nil, errors.Trace(ErrIncompatibleTiFlashAndPlacement) + } + // bundle for table should be recomputed because it includes some default configs for partitions tblBundle, err := placement.NewTableBundle(t, tblInfo) if err != nil {