From bff70226c2bd66e2524a28e035c6a9dfacca0547 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 11:42:41 +0800 Subject: [PATCH 01/11] *: add split table syntax to split table region --- executor/builder.go | 19 ++++-- executor/executor_test.go | 37 +++++++++- executor/split.go | 103 ++++++++++++++++++++++++++++ executor/split_test.go | 72 ++++++++++++++++++++ planner/core/planbuilder.go | 130 ++++++++++++++++++++++++++++-------- 5 files changed, 327 insertions(+), 34 deletions(-) diff --git a/executor/builder.go b/executor/builder.go index a55bad1f91170..c52cf4d6e58e7 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -195,7 +195,7 @@ func (b *executorBuilder) build(p plannercore.Plan) Executor { case *plannercore.SQLBindPlan: return b.buildSQLBindExec(v) case *plannercore.SplitRegion: - return b.buildSplitIndexRegion(v) + return b.buildSplitRegion(v) default: if mp, ok := p.(MockPhysicalPlan); ok { return mp.GetExecutor() @@ -1261,18 +1261,29 @@ func (b *executorBuilder) buildUnionAll(v *plannercore.PhysicalUnionAll) Executo return e } -func (b *executorBuilder) buildSplitIndexRegion(v *plannercore.SplitRegion) Executor { +func (b *executorBuilder) buildSplitRegion(v *plannercore.SplitRegion) Executor { base := newBaseExecutor(b.ctx, nil, v.ExplainID()) base.initCap = chunk.ZeroCapacity - return &SplitIndexRegionExec{ + if v.IndexInfo != nil { + return &SplitIndexRegionExec{ + baseExecutor: base, + tableInfo: v.TableInfo, + indexInfo: v.IndexInfo, + lower: v.Lower, + upper: v.Upper, + num: v.Num, + valueLists: v.ValueLists, + } + } + return &SplitTableRegionExec{ baseExecutor: base, tableInfo: v.TableInfo, - indexInfo: v.IndexInfo, lower: v.Lower, upper: v.Upper, num: v.Num, valueLists: v.ValueLists, } + } func (b *executorBuilder) buildUpdate(v *plannercore.Update) Executor { diff --git a/executor/executor_test.go b/executor/executor_test.go index 2f733c7e00edb..060e55edc9171 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3791,7 +3791,7 @@ func (s *testSuite) TestReadPartitionedTable(c *C) { tk.MustQuery("select a from pt where b = 3").Check(testkit.Rows("3")) } -func (s *testSuite) TestSplitIndexRegion(c *C) { +func (s *testSuite) TestSplitRegion(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("drop table if exists t") @@ -3802,6 +3802,7 @@ func (s *testSuite) TestSplitIndexRegion(c *C) { terr := errors.Cause(err).(*terror.Error) c.Assert(terr.Code(), Equals, terror.ErrCode(mysql.WarnDataTruncated)) + // Test for split index region. // Check min value is more than max value. tk.MustExec(`split table t index idx1 between (0) and (1000000000) regions 10`) _, err = tk.Exec(`split table t index idx1 between (2,'a') and (1,'c') regions 10`) @@ -3831,7 +3832,39 @@ func (s *testSuite) TestSplitIndexRegion(c *C) { // Test truncate error msg. _, err = tk.Exec(`split table t index idx1 between ("aa") and (1000000000) regions 0`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "[types:1265]Incorrect value: 'aa' for index column 'b'") + c.Assert(err.Error(), Equals, "[types:1265]Incorrect value: 'aa' for column 'b'") + + // Test for split table region. + tk.MustExec(`split table t between (0) and (1000000000) regions 10`) + // Check min value is more than upper value. + _, err = tk.Exec(`split table t between (2) and (1) regions 10`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table t region lower value 2 should less than the upper value 1") + + // Check min value is invalid. + _, err = tk.Exec(`split table t between () and (1) regions 10`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table region lower value count should be 1") + + // Check upper value is invalid. + _, err = tk.Exec(`split table t between (1) and () regions 10`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table region upper value count should be 1") + + // Check pre-split region num is too large. + _, err = tk.Exec(`split table t between (0) and (1000000000) regions 10000`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table region num is exceed the limit 1000") + + // Check pre-split region num 0 is invalid. + _, err = tk.Exec(`split table t between (0) and (1000000000) regions 0`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table region num should more than 0") + + // Test truncate error msg. + _, err = tk.Exec(`split table t between ("aa") and (1000000000) regions 10`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[types:1265]Incorrect value: 'aa' for column '_tidb_rowid'") } func (s *testSuite) TestIssue10435(c *C) { diff --git a/executor/split.go b/executor/split.go index 255caaf94b118..9b2f6bbf59db4 100644 --- a/executor/split.go +++ b/executor/split.go @@ -22,6 +22,7 @@ import ( "github.com/cznic/mathutil" "github.com/pingcap/errors" "github.com/pingcap/parser/model" + "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" @@ -204,3 +205,105 @@ func datumSliceToString(ds []types.Datum) (string, error) { str += ")" return str, nil } + +// SplitTableRegionExec represents a split table regions executor. +type SplitTableRegionExec struct { + baseExecutor + + tableInfo *model.TableInfo + lower []types.Datum + upper []types.Datum + num int + valueLists [][]types.Datum +} + +// Next implements the Executor Next interface. +func (e *SplitTableRegionExec) Next(ctx context.Context, _ *chunk.RecordBatch) error { + store := e.ctx.GetStore() + s, ok := store.(splitableStore) + if !ok { + return nil + } + splitKeys, err := e.getSplitTableKeys() + if err != nil { + return err + } + regionIDs := make([]uint64, 0, len(splitKeys)) + for _, idxKey := range splitKeys { + regionID, err := s.SplitRegionAndScatter(idxKey) + if err != nil { + logutil.Logger(context.Background()).Warn("split table region failed", + zap.String("table", e.tableInfo.Name.L), + zap.Error(err)) + continue + } + regionIDs = append(regionIDs, regionID) + + } + if !e.ctx.GetSessionVars().WaitTableSplitFinish { + return nil + } + for _, regionID := range regionIDs { + err := s.WaitScatterRegionFinish(regionID) + if err != nil { + logutil.Logger(context.Background()).Warn("wait scatter region failed", + zap.Uint64("regionID", regionID), + zap.String("table", e.tableInfo.Name.L), + zap.Error(err)) + } + } + return nil +} + +func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { + var keys [][]byte + if e.num > 0 { + keys = make([][]byte, 0, e.num) + } else { + keys = make([][]byte, 0, len(e.valueLists)) + } + recordPrefix := tablecodec.GenTableRecordPrefix(e.tableInfo.ID) + if len(e.valueLists) > 0 { + for _, v := range e.valueLists { + key := tablecodec.EncodeRecordKey(recordPrefix, v[0].GetInt64()) + keys = append(keys, key) + } + return keys, nil + } + isUnsigned := false + if e.tableInfo.PKIsHandle { + if pkCol := e.tableInfo.GetPkColInfo(); pkCol != nil { + isUnsigned = mysql.HasUnsignedFlag(pkCol.Flag) + } + } + step := uint64(0) + lowerValue := int64(0) + if isUnsigned { + lowerRecordID := e.lower[0].GetUint64() + upperRecordID := e.upper[0].GetUint64() + if upperRecordID <= lowerRecordID { + return nil, errors.Errorf("Split table %s region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) + } + step = (upperRecordID - lowerRecordID) / uint64(e.num) + lowerValue = int64(lowerRecordID) + } else { + lowerRecordID := e.lower[0].GetInt64() + upperRecordID := e.upper[0].GetInt64() + if upperRecordID <= lowerRecordID { + return nil, errors.Errorf("Split table %s region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) + } + step = uint64(upperRecordID-lowerRecordID) / uint64(e.num) + lowerValue = lowerRecordID + } + if step < 1 { + return nil, errors.Errorf("Split table %s region step value should more than 0, step %v is invalid", e.tableInfo.Name, step) + } + + recordID := lowerValue + for i := 1; i < e.num; i++ { + recordID += int64(step) + key := tablecodec.EncodeRecordKey(recordPrefix, recordID) + keys = append(keys, key) + } + return keys, nil +} diff --git a/executor/split_test.go b/executor/split_test.go index c73d37a5d65dd..5d881b60d5a2e 100644 --- a/executor/split_test.go +++ b/executor/split_test.go @@ -16,6 +16,7 @@ package executor import ( "bytes" "encoding/binary" + "github.com/pingcap/tidb/tablecodec" "math" "math/rand" @@ -285,6 +286,77 @@ func (s *testSplitIndex) TestSplitIndex(c *C) { } } +func (s *testSplitIndex) TestSplitTable(c *C) { + tbInfo := &model.TableInfo{ + Name: model.NewCIStr("t1"), + ID: rand.Int63(), + Columns: []*model.ColumnInfo{ + { + Name: model.NewCIStr("c0"), + ID: 1, + Offset: 1, + DefaultValue: 0, + State: model.StatePublic, + FieldType: *types.NewFieldType(mysql.TypeLong), + }, + }, + } + // range is 0 ~ 100, and split into 10 region. + // So 10 regions range is like below: + // region1: -inf ~ 10 + // region2: [10 ~ 20) + // region3: [20 ~ 30) + // region4: [30 ~ 40) + // region5: [40 ~ 50) + // region6: [50 ~ 60) + // region7: [60 ~ 70) + // region8: [70 ~ 80) + // region9: [80 ~ 90 ) + // region10: [90 ~ +inf) + ctx := mock.NewContext() + e := &SplitTableRegionExec{ + baseExecutor: newBaseExecutor(ctx, nil, nil), + tableInfo: tbInfo, + lower: []types.Datum{types.NewDatum(0)}, + upper: []types.Datum{types.NewDatum(100)}, + num: 10, + } + valueList, err := e.getSplitTableKeys() + c.Assert(err, IsNil) + c.Assert(len(valueList), Equals, e.num-1) + + cases := []struct { + value int + lessEqualIdx int + }{ + {-1, -1}, + {0, -1}, + {1, -1}, + {10, 0}, + {11, 0}, + {20, 1}, + {21, 1}, + {31, 2}, + {41, 3}, + {51, 4}, + {61, 5}, + {71, 6}, + {81, 7}, + {91, 8}, + {100, 8}, + {1000, 8}, + } + + recordPrefix := tablecodec.GenTableRecordPrefix(e.tableInfo.ID) + for _, ca := range cases { + // test for minInt64 handle + key := tablecodec.EncodeRecordKey(recordPrefix, int64(ca.value)) + c.Assert(err, IsNil) + idx := searchLessEqualIdx(valueList, key) + c.Assert(idx, Equals, ca.lessEqualIdx, Commentf("%#v", ca)) + } +} + func searchLessEqualIdx(valueList [][]byte, value []byte) int { idx := -1 for i, v := range valueList { diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 117aa12a80c01..e5008d5cab258 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1623,6 +1623,13 @@ func (b *PlanBuilder) buildLoadStats(ld *ast.LoadStatsStmt) Plan { const maxSplitRegionNum = 1000 func (b *PlanBuilder) buildSplitRegion(node *ast.SplitRegionStmt) (Plan, error) { + if len(node.IndexName.L) != 0 { + return b.buildSplitIndexRegion(node) + } + return b.buildSplitTableRegion(node) +} + +func (b *PlanBuilder) buildSplitIndexRegion(node *ast.SplitRegionStmt) (Plan, error) { tblInfo := node.Table.TableInfo indexInfo := tblInfo.FindIndexByName(node.IndexName.L) if indexInfo == nil { @@ -1686,43 +1693,110 @@ func (b *PlanBuilder) buildSplitRegion(node *ast.SplitRegionStmt) (Plan, error) func (b *PlanBuilder) convertValue2ColumnType(valuesItem []ast.ExprNode, mockTablePlan LogicalPlan, indexInfo *model.IndexInfo, tblInfo *model.TableInfo) ([]types.Datum, error) { values := make([]types.Datum, 0, len(valuesItem)) for j, valueItem := range valuesItem { - var expr expression.Expression - var err error - switch x := valueItem.(type) { - case *driver.ValueExpr: - expr = &expression.Constant{ - Value: x.Datum, - RetType: &x.Type, - } - default: - expr, _, err = b.rewrite(valueItem, mockTablePlan, nil, true) - if err != nil { - return nil, err - } - } - constant, ok := expr.(*expression.Constant) - if !ok { - return nil, errors.New("expect constant values") - } - value, err := constant.Eval(chunk.Row{}) + colOffset := indexInfo.Columns[j].Offset + value, err := b.convertValue(valueItem, mockTablePlan, tblInfo.Columns[colOffset]) if err != nil { return nil, err } - colOffset := indexInfo.Columns[j].Offset - value1, err := value.ConvertTo(b.ctx.GetSessionVars().StmtCtx, &tblInfo.Columns[colOffset].FieldType) + values = append(values, value) + } + return values, nil +} + +func (b *PlanBuilder) convertValue(valueItem ast.ExprNode, mockTablePlan LogicalPlan, col *model.ColumnInfo) (d types.Datum, err error) { + var expr expression.Expression + switch x := valueItem.(type) { + case *driver.ValueExpr: + expr = &expression.Constant{ + Value: x.Datum, + RetType: &x.Type, + } + default: + expr, _, err = b.rewrite(valueItem, mockTablePlan, nil, true) if err != nil { - if !types.ErrTruncated.Equal(err) { - return nil, err + return d, err + } + } + constant, ok := expr.(*expression.Constant) + if !ok { + return d, errors.New("expect constant values") + } + value, err := constant.Eval(chunk.Row{}) + if err != nil { + return d, err + } + d, err = value.ConvertTo(b.ctx.GetSessionVars().StmtCtx, &col.FieldType) + if err != nil { + if !types.ErrTruncated.Equal(err) { + return d, err + } + valStr, err1 := value.ToString() + if err1 != nil { + return d, err + } + return d, types.ErrTruncated.GenWithStack("Incorrect value: '%-.128s' for column '%.192s'", valStr, col.Name.O) + } + return d, nil +} + +func (b *PlanBuilder) buildSplitTableRegion(node *ast.SplitRegionStmt) (Plan, error) { + tblInfo := node.Table.TableInfo + var pkCol *model.ColumnInfo + if tblInfo.PKIsHandle { + if col := tblInfo.GetPkColInfo(); col != nil { + pkCol = col + } + } + if pkCol == nil { + pkCol = model.NewExtraHandleColInfo() + } + mockTablePlan := LogicalTableDual{}.Init(b.ctx) + schema := expression.TableInfo2SchemaWithDBName(b.ctx, node.Table.Schema, tblInfo) + mockTablePlan.SetSchema(schema) + + p := &SplitRegion{ + TableInfo: tblInfo, + } + if len(node.SplitOpt.ValueLists) > 0 { + values := make([][]types.Datum, 0, len(node.SplitOpt.ValueLists)) + for i, valuesItem := range node.SplitOpt.ValueLists { + if len(valuesItem) > 1 { + return nil, ErrWrongValueCountOnRow.GenWithStackByArgs(i + 1) } - valStr, err1 := value.ToString() - if err1 != nil { + value, err := b.convertValue(valuesItem[0], mockTablePlan, pkCol) + if err != nil { return nil, err } - return nil, types.ErrTruncated.GenWithStack("Incorrect value: '%-.128s' for index column '%.192s'", valStr, tblInfo.Columns[colOffset].Name.O) + values = append(values, []types.Datum{value}) } - values = append(values, value1) + p.ValueLists = values + return p, nil } - return values, nil + + checkLowerUpperValue := func(valuesItem []ast.ExprNode, name string) (types.Datum, error) { + if len(valuesItem) != 1 { + return types.Datum{}, errors.Errorf("Split table region %s value count should be 1", name) + } + return b.convertValue(valuesItem[0], mockTablePlan, pkCol) + } + lowerValues, err := checkLowerUpperValue(node.SplitOpt.Lower, "lower") + if err != nil { + return nil, err + } + upperValue, err := checkLowerUpperValue(node.SplitOpt.Upper, "upper") + if err != nil { + return nil, err + } + p.Lower = []types.Datum{lowerValues} + p.Upper = []types.Datum{upperValue} + + if node.SplitOpt.Num > maxSplitRegionNum { + return nil, errors.Errorf("Split table region num is exceed the limit %v", maxSplitRegionNum) + } else if node.SplitOpt.Num < 1 { + return nil, errors.Errorf("Split table region num should more than 0") + } + p.Num = int(node.SplitOpt.Num) + return p, nil } func (b *PlanBuilder) buildDDL(node ast.DDLNode) (Plan, error) { From 3cd169e4bb8065aa3e51b91910761bf3929a88e7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 11:54:15 +0800 Subject: [PATCH 02/11] refine code --- executor/builder.go | 1 - executor/executor_test.go | 5 +++++ executor/split.go | 6 ++++-- executor/split_test.go | 4 ++-- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/executor/builder.go b/executor/builder.go index c52cf4d6e58e7..e5f3efa64485d 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -1283,7 +1283,6 @@ func (b *executorBuilder) buildSplitRegion(v *plannercore.SplitRegion) Executor num: v.Num, valueLists: v.ValueLists, } - } func (b *executorBuilder) buildUpdate(v *plannercore.Update) Executor { diff --git a/executor/executor_test.go b/executor/executor_test.go index 060e55edc9171..4a3b7a7476320 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3865,6 +3865,11 @@ func (s *testSuite) TestSplitRegion(c *C) { _, err = tk.Exec(`split table t between ("aa") and (1000000000) regions 10`) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[types:1265]Incorrect value: 'aa' for column '_tidb_rowid'") + + // Test split table region step is too small. + _, err = tk.Exec(`split table t between (0) and (100) regions 10`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "Split table t region step value should more than 1000, step 10 is invalid") } func (s *testSuite) TestIssue10435(c *C) { diff --git a/executor/split.go b/executor/split.go index 9b2f6bbf59db4..3f34e0469543c 100644 --- a/executor/split.go +++ b/executor/split.go @@ -255,6 +255,8 @@ func (e *SplitTableRegionExec) Next(ctx context.Context, _ *chunk.RecordBatch) e return nil } +const minRegionStepValue = 1000 + func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { var keys [][]byte if e.num > 0 { @@ -295,8 +297,8 @@ func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { step = uint64(upperRecordID-lowerRecordID) / uint64(e.num) lowerValue = lowerRecordID } - if step < 1 { - return nil, errors.Errorf("Split table %s region step value should more than 0, step %v is invalid", e.tableInfo.Name, step) + if step < minRegionStepValue { + return nil, errors.Errorf("Split table %s region step value should more than %v, step %v is invalid", e.tableInfo.Name, minRegionStepValue, step) } recordID := lowerValue diff --git a/executor/split_test.go b/executor/split_test.go index 5d881b60d5a2e..2c9a0c0fb812f 100644 --- a/executor/split_test.go +++ b/executor/split_test.go @@ -16,7 +16,6 @@ package executor import ( "bytes" "encoding/binary" - "github.com/pingcap/tidb/tablecodec" "math" "math/rand" @@ -24,6 +23,7 @@ import ( "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/table/tables" + "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/mock" ) @@ -303,7 +303,7 @@ func (s *testSplitIndex) TestSplitTable(c *C) { } // range is 0 ~ 100, and split into 10 region. // So 10 regions range is like below: - // region1: -inf ~ 10 + // region1: [-inf ~ 10) // region2: [10 ~ 20) // region3: [20 ~ 30) // region4: [30 ~ 40) From da3a46ecb24a1ea5039bd6d338ed142323ff18ab Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 6 Jun 2019 14:03:54 +0800 Subject: [PATCH 03/11] Update executor/executor_test.go Co-Authored-By: bb7133 --- executor/executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 4a3b7a7476320..b35350d9beeb8 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3836,7 +3836,7 @@ func (s *testSuite) TestSplitRegion(c *C) { // Test for split table region. tk.MustExec(`split table t between (0) and (1000000000) regions 10`) - // Check min value is more than upper value. + // Check the ower value is more than the upper value. _, err = tk.Exec(`split table t between (2) and (1) regions 10`) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "Split table t region lower value 2 should less than the upper value 1") From 4ad1cc984cbfd5546d7ef62f394e3f47403a0699 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 6 Jun 2019 14:04:03 +0800 Subject: [PATCH 04/11] Update executor/executor_test.go Co-Authored-By: bb7133 --- executor/executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index b35350d9beeb8..5e3c9ebe7c1ad 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3841,7 +3841,7 @@ func (s *testSuite) TestSplitRegion(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "Split table t region lower value 2 should less than the upper value 1") - // Check min value is invalid. + // Check the lower value is invalid. _, err = tk.Exec(`split table t between () and (1) regions 10`) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "Split table region lower value count should be 1") From 6ef83923cae85fd0e9adc48dc5bed7bc7d154067 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 6 Jun 2019 14:04:22 +0800 Subject: [PATCH 05/11] Update planner/core/planbuilder.go Co-Authored-By: bb7133 --- planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index e5008d5cab258..54319138f107a 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1719,7 +1719,7 @@ func (b *PlanBuilder) convertValue(valueItem ast.ExprNode, mockTablePlan Logical } constant, ok := expr.(*expression.Constant) if !ok { - return d, errors.New("expect constant values") + return d, errors.New("Expect constant values.") } value, err := constant.Eval(chunk.Row{}) if err != nil { From a10e2ba76613362cdd71301020de59767cc4518d Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 17:00:29 +0800 Subject: [PATCH 06/11] address comment --- executor/split.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/split.go b/executor/split.go index 3f34e0469543c..5c127f65e3b25 100644 --- a/executor/split.go +++ b/executor/split.go @@ -229,8 +229,8 @@ func (e *SplitTableRegionExec) Next(ctx context.Context, _ *chunk.RecordBatch) e return err } regionIDs := make([]uint64, 0, len(splitKeys)) - for _, idxKey := range splitKeys { - regionID, err := s.SplitRegionAndScatter(idxKey) + for _, key := range splitKeys { + regionID, err := s.SplitRegionAndScatter(key) if err != nil { logutil.Logger(context.Background()).Warn("split table region failed", zap.String("table", e.tableInfo.Name.L), From 6e6ec4b8669f3a328dcbd72df38c0c9f91f3c591 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 17:52:29 +0800 Subject: [PATCH 07/11] refine error msg and fix msg --- executor/executor_test.go | 14 +++++++------- executor/split.go | 12 ++++++------ executor/split_test.go | 4 ++++ planner/core/planbuilder.go | 8 ++++---- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 5e3c9ebe7c1ad..87a8356cb4566 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3807,22 +3807,22 @@ func (s *testSuite) TestSplitRegion(c *C) { tk.MustExec(`split table t index idx1 between (0) and (1000000000) regions 10`) _, err = tk.Exec(`split table t index idx1 between (2,'a') and (1,'c') regions 10`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split index region `idx1` lower value (2,a) should less than the upper value (1,c)") + c.Assert(err.Error(), Equals, "Split index `idx1` region lower value (2,a) should less than the upper value (1,c)") // Check min value is invalid. _, err = tk.Exec(`split table t index idx1 between () and (1) regions 10`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split index region `idx1` lower value count should more than 0") + c.Assert(err.Error(), Equals, "Split index `idx1` region lower value count should more than 0") // Check max value is invalid. _, err = tk.Exec(`split table t index idx1 between (1) and () regions 10`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split index region `idx1` upper value count should more than 0") + c.Assert(err.Error(), Equals, "Split index `idx1` region upper value count should more than 0") // Check pre-split region num is too large. _, err = tk.Exec(`split table t index idx1 between (0) and (1000000000) regions 10000`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split index region num is exceed the limit 1000") + c.Assert(err.Error(), Equals, "Split index region num exceeded the limit 1000") // Check pre-split region num 0 is invalid. _, err = tk.Exec(`split table t index idx1 between (0) and (1000000000) regions 0`) @@ -3839,7 +3839,7 @@ func (s *testSuite) TestSplitRegion(c *C) { // Check the ower value is more than the upper value. _, err = tk.Exec(`split table t between (2) and (1) regions 10`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split table t region lower value 2 should less than the upper value 1") + c.Assert(err.Error(), Equals, "Split table `t` region lower value 2 should less than the upper value 1") // Check the lower value is invalid. _, err = tk.Exec(`split table t between () and (1) regions 10`) @@ -3854,7 +3854,7 @@ func (s *testSuite) TestSplitRegion(c *C) { // Check pre-split region num is too large. _, err = tk.Exec(`split table t between (0) and (1000000000) regions 10000`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split table region num is exceed the limit 1000") + c.Assert(err.Error(), Equals, "Split table region num exceeded the limit 1000") // Check pre-split region num 0 is invalid. _, err = tk.Exec(`split table t between (0) and (1000000000) regions 0`) @@ -3869,7 +3869,7 @@ func (s *testSuite) TestSplitRegion(c *C) { // Test split table region step is too small. _, err = tk.Exec(`split table t between (0) and (100) regions 10`) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "Split table t region step value should more than 1000, step 10 is invalid") + c.Assert(err.Error(), Equals, "Split table `t` region step value should more than 1000, step 10 is invalid") } func (s *testSuite) TestIssue10435(c *C) { diff --git a/executor/split.go b/executor/split.go index 5c127f65e3b25..8f334a84e9410 100644 --- a/executor/split.go +++ b/executor/split.go @@ -127,9 +127,9 @@ func (e *SplitIndexRegionExec) getSplitIdxKeys() ([][]byte, error) { lowerStr, err1 := datumSliceToString(e.lower) upperStr, err2 := datumSliceToString(e.upper) if err1 != nil || err2 != nil { - return nil, errors.Errorf("Split index region `%v` lower value %v should less than the upper value %v", e.indexInfo.Name, e.lower, e.upper) + return nil, errors.Errorf("Split index `%v` region lower value %v should less than the upper value %v", e.indexInfo.Name, e.lower, e.upper) } - return nil, errors.Errorf("Split index region `%v` lower value %v should less than the upper value %v", e.indexInfo.Name, lowerStr, upperStr) + return nil, errors.Errorf("Split index `%v` region lower value %v should less than the upper value %v", e.indexInfo.Name, lowerStr, upperStr) } return getValuesList(lowerIdxKey, upperIdxKey, e.num, idxKeys), nil } @@ -255,7 +255,7 @@ func (e *SplitTableRegionExec) Next(ctx context.Context, _ *chunk.RecordBatch) e return nil } -const minRegionStepValue = 1000 +var minRegionStepValue = uint64(1000) func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { var keys [][]byte @@ -284,7 +284,7 @@ func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { lowerRecordID := e.lower[0].GetUint64() upperRecordID := e.upper[0].GetUint64() if upperRecordID <= lowerRecordID { - return nil, errors.Errorf("Split table %s region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) + return nil, errors.Errorf("Split table `%s` region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) } step = (upperRecordID - lowerRecordID) / uint64(e.num) lowerValue = int64(lowerRecordID) @@ -292,13 +292,13 @@ func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { lowerRecordID := e.lower[0].GetInt64() upperRecordID := e.upper[0].GetInt64() if upperRecordID <= lowerRecordID { - return nil, errors.Errorf("Split table %s region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) + return nil, errors.Errorf("Split table `%s` region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) } step = uint64(upperRecordID-lowerRecordID) / uint64(e.num) lowerValue = lowerRecordID } if step < minRegionStepValue { - return nil, errors.Errorf("Split table %s region step value should more than %v, step %v is invalid", e.tableInfo.Name, minRegionStepValue, step) + return nil, errors.Errorf("Split table `%s` region step value should more than %v, step %v is invalid", e.tableInfo.Name, minRegionStepValue, step) } recordID := lowerValue diff --git a/executor/split_test.go b/executor/split_test.go index 2c9a0c0fb812f..7a35fdfa304c8 100644 --- a/executor/split_test.go +++ b/executor/split_test.go @@ -301,6 +301,10 @@ func (s *testSplitIndex) TestSplitTable(c *C) { }, }, } + defer func(originValue uint64) { + minRegionStepValue = originValue + }(minRegionStepValue) + minRegionStepValue = 10 // range is 0 ~ 100, and split into 10 region. // So 10 regions range is like below: // region1: [-inf ~ 10) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 54319138f107a..b27dd3321d5e0 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1663,10 +1663,10 @@ func (b *PlanBuilder) buildSplitIndexRegion(node *ast.SplitRegionStmt) (Plan, er // Split index regions by lower, upper value. checkLowerUpperValue := func(valuesItem []ast.ExprNode, name string) ([]types.Datum, error) { if len(valuesItem) == 0 { - return nil, errors.Errorf("Split index region `%v` %s value count should more than 0", indexInfo.Name, name) + return nil, errors.Errorf("Split index `%v` region %s value count should more than 0", indexInfo.Name, name) } if len(valuesItem) > len(indexInfo.Columns) { - return nil, errors.Errorf("Split index region `%v` Column count doesn't match value count at %v", indexInfo.Name, name) + return nil, errors.Errorf("Split index `%v` region column count doesn't match value count at %v", indexInfo.Name, name) } return b.convertValue2ColumnType(valuesItem, mockTablePlan, indexInfo, tblInfo) } @@ -1682,7 +1682,7 @@ func (b *PlanBuilder) buildSplitIndexRegion(node *ast.SplitRegionStmt) (Plan, er p.Upper = upperValues if node.SplitOpt.Num > maxSplitRegionNum { - return nil, errors.Errorf("Split index region num is exceed the limit %v", maxSplitRegionNum) + return nil, errors.Errorf("Split index region num exceeded the limit %v", maxSplitRegionNum) } else if node.SplitOpt.Num < 1 { return nil, errors.Errorf("Split index region num should more than 0") } @@ -1791,7 +1791,7 @@ func (b *PlanBuilder) buildSplitTableRegion(node *ast.SplitRegionStmt) (Plan, er p.Upper = []types.Datum{upperValue} if node.SplitOpt.Num > maxSplitRegionNum { - return nil, errors.Errorf("Split table region num is exceed the limit %v", maxSplitRegionNum) + return nil, errors.Errorf("Split table region num exceeded the limit %v", maxSplitRegionNum) } else if node.SplitOpt.Num < 1 { return nil, errors.Errorf("Split table region num should more than 0") } From 874eb02959e2856d4e3fd140f80dc7cc118a4069 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 17:57:44 +0800 Subject: [PATCH 08/11] fix ci --- planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index b27dd3321d5e0..155e9758e4d51 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1719,7 +1719,7 @@ func (b *PlanBuilder) convertValue(valueItem ast.ExprNode, mockTablePlan Logical } constant, ok := expr.(*expression.Constant) if !ok { - return d, errors.New("Expect constant values.") + return d, errors.New("Expect constant values") } value, err := constant.Eval(chunk.Row{}) if err != nil { From a4a279c81ff523b104f6deb63017e7f1cf93d5b6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 6 Jun 2019 19:46:46 +0800 Subject: [PATCH 09/11] address comment --- executor/builder.go | 4 ++-- executor/split.go | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/executor/builder.go b/executor/builder.go index e5f3efa64485d..1cebacbf2fcd9 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -1278,8 +1278,8 @@ func (b *executorBuilder) buildSplitRegion(v *plannercore.SplitRegion) Executor return &SplitTableRegionExec{ baseExecutor: base, tableInfo: v.TableInfo, - lower: v.Lower, - upper: v.Upper, + lower: v.Lower[0], + upper: v.Upper[0], num: v.Num, valueLists: v.ValueLists, } diff --git a/executor/split.go b/executor/split.go index 8f334a84e9410..29d54dcd70288 100644 --- a/executor/split.go +++ b/executor/split.go @@ -211,8 +211,8 @@ type SplitTableRegionExec struct { baseExecutor tableInfo *model.TableInfo - lower []types.Datum - upper []types.Datum + lower types.Datum + upper types.Datum num int valueLists [][]types.Datum } @@ -278,19 +278,19 @@ func (e *SplitTableRegionExec) getSplitTableKeys() ([][]byte, error) { isUnsigned = mysql.HasUnsignedFlag(pkCol.Flag) } } - step := uint64(0) - lowerValue := int64(0) + var step uint64 + var lowerValue int64 if isUnsigned { - lowerRecordID := e.lower[0].GetUint64() - upperRecordID := e.upper[0].GetUint64() + lowerRecordID := e.lower.GetUint64() + upperRecordID := e.upper.GetUint64() if upperRecordID <= lowerRecordID { return nil, errors.Errorf("Split table `%s` region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) } step = (upperRecordID - lowerRecordID) / uint64(e.num) lowerValue = int64(lowerRecordID) } else { - lowerRecordID := e.lower[0].GetInt64() - upperRecordID := e.upper[0].GetInt64() + lowerRecordID := e.lower.GetInt64() + upperRecordID := e.upper.GetInt64() if upperRecordID <= lowerRecordID { return nil, errors.Errorf("Split table `%s` region lower value %v should less than the upper value %v", e.tableInfo.Name, lowerRecordID, upperRecordID) } From 2cf842f40c8c774e12abde143701cd983c5f8bb7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 10 Jun 2019 09:38:38 +0800 Subject: [PATCH 10/11] fix test --- executor/split_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/split_test.go b/executor/split_test.go index 7a35fdfa304c8..c090faf20d3c7 100644 --- a/executor/split_test.go +++ b/executor/split_test.go @@ -321,8 +321,8 @@ func (s *testSplitIndex) TestSplitTable(c *C) { e := &SplitTableRegionExec{ baseExecutor: newBaseExecutor(ctx, nil, nil), tableInfo: tbInfo, - lower: []types.Datum{types.NewDatum(0)}, - upper: []types.Datum{types.NewDatum(100)}, + lower: types.NewDatum(0), + upper: types.NewDatum(100), num: 10, } valueList, err := e.getSplitTableKeys() From 8f939ae543ff9bd4b526b8e0f904459019113210 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 10 Jun 2019 14:31:57 +0800 Subject: [PATCH 11/11] gs --- executor/split.go | 1 - 1 file changed, 1 deletion(-) diff --git a/executor/split.go b/executor/split.go index 29d54dcd70288..586cfe1ad749b 100644 --- a/executor/split.go +++ b/executor/split.go @@ -238,7 +238,6 @@ func (e *SplitTableRegionExec) Next(ctx context.Context, _ *chunk.RecordBatch) e continue } regionIDs = append(regionIDs, regionID) - } if !e.ctx.GetSessionVars().WaitTableSplitFinish { return nil