From 7f2216d170aa2149ed649b0e62cd69126c311c0e Mon Sep 17 00:00:00 2001 From: Arenatlx Date: Fri, 21 Aug 2020 14:32:52 +0800 Subject: [PATCH 1/2] cherry pick #19330 to release-4.0 Signed-off-by: ti-srebot --- ddl/db_test.go | 231 +++++++++++++++++++++++++++++++++++++++++++++++++ ddl/index.go | 17 ++++ 2 files changed, 248 insertions(+) diff --git a/ddl/db_test.go b/ddl/db_test.go index 2a252c24edcfb..425bdaecb19ca 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -4676,6 +4676,237 @@ func (s *testSerialDBSuite) TestDDLJobErrorCount(c *C) { } } +<<<<<<< HEAD +======= +func (s *testDBSuite1) TestAlterTableWithValidation(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t1") + defer tk.MustExec("drop table if exists t1") + + tk.MustExec("create table t1 (c1 int, c2 int as (c1 + 1));") + + // Test for alter table with validation. + tk.MustExec("alter table t1 with validation") + c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(1)) + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|8200|ALTER TABLE WITH VALIDATION is currently unsupported")) + + // Test for alter table without validation. + tk.MustExec("alter table t1 without validation") + c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(1)) + tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|8200|ALTER TABLE WITHOUT VALIDATION is currently unsupported")) +} + +func (s *testSerialDBSuite) TestCommitTxnWithIndexChange(c *C) { + // Prepare work. + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("drop database if exists test_db") + tk.MustExec("create database test_db") + tk.MustExec("use test_db") + tk.MustExec("create table t1 (c1 int primary key, c2 int, c3 int, index ok2(c2))") + tk.MustExec("insert t1 values (1, 10, 100), (2, 20, 200)") + tk.MustExec("alter table t1 add index k2(c2)") + tk.MustExec("alter table t1 drop index k2") + tk.MustExec("alter table t1 add index k2(c2)") + tk.MustExec("alter table t1 drop index k2") + tk2 := testkit.NewTestKit(c, s.store) + tk2.MustExec("use test_db") + + // tkSQLs are the sql statements for the pessimistic transaction. + // tk2DDL are the ddl statements executed before the pessimistic transaction. + // idxDDL is the DDL statement executed between pessimistic transaction begin and commit. + // failCommit means the pessimistic transaction commit should fail not. + type caseUnit struct { + tkSQLs []string + tk2DDL []string + idxDDL string + checkSQLs []string + rowsExps [][]string + failCommit bool + stateEnd model.SchemaState + } + + cases := []caseUnit{ + // Test secondary index + {[]string{"insert into t1 values(3, 30, 300)", + "insert into t2 values(11, 11, 11)"}, + []string{"alter table t1 add index k2(c2)", + "alter table t1 drop index k2", + "alter table t1 add index kk2(c2, c1)", + "alter table t1 add index k2(c2)", + "alter table t1 drop index k2"}, + "alter table t1 add index k2(c2)", + []string{"select c3, c2 from t1 use index(k2) where c2 = 20", + "select c3, c2 from t1 use index(k2) where c2 = 10", + "select * from t1", + "select * from t2 where c1 = 11"}, + [][]string{{"200 20"}, + {"100 10"}, + {"1 10 100", "2 20 200", "3 30 300"}, + {"11 11 11"}}, + false, + model.StateNone}, + // Test secondary index + {[]string{"insert into t2 values(5, 50, 500)", + "insert into t2 values(11, 11, 11)", + "delete from t2 where c2 = 11", + "update t2 set c2 = 110 where c1 = 11"}, + //"update t2 set c1 = 10 where c3 = 100"}, + []string{"alter table t1 add index k2(c2)", + "alter table t1 drop index k2", + "alter table t1 add index kk2(c2, c1)", + "alter table t1 add index k2(c2)", + "alter table t1 drop index k2"}, + "alter table t1 add index k2(c2)", + []string{"select c3, c2 from t1 use index(k2) where c2 = 20", + "select c3, c2 from t1 use index(k2) where c2 = 10", + "select * from t1", + "select * from t2 where c1 = 11", + "select * from t2 where c3 = 100"}, + [][]string{{"200 20"}, + {"100 10"}, + {"1 10 100", "2 20 200"}, + {}, + {"1 10 100"}}, + false, + model.StateNone}, + // Test unique index + /* TODO unique index is not supported now. + {[]string{"insert into t1 values(3, 30, 300)", + "insert into t1 values(4, 40, 400)", + "insert into t2 values(11, 11, 11)", + "insert into t2 values(12, 12, 11)"}, + []string{"alter table t1 add unique index uk3(c3)", + "alter table t1 drop index uk3", + "alter table t2 add unique index ukc1c3(c1, c3)", + "alter table t2 add unique index ukc3(c3)", + "alter table t2 drop index ukc1c3", + "alter table t2 drop index ukc3", + "alter table t2 add index kc3(c3)"}, + "alter table t1 add unique index uk3(c3)", + []string{"select c3, c2 from t1 use index(uk3) where c3 = 200", + "select c3, c2 from t1 use index(uk3) where c3 = 300", + "select c3, c2 from t1 use index(uk3) where c3 = 400", + "select * from t1", + "select * from t2"}, + [][]string{{"200 20"}, + {"300 30"}, + {"400 40"}, + {"1 10 100", "2 20 200", "3 30 300", "4 40 400"}, + {"1 10 100", "2 20 200", "11 11 11", "12 12 11"}}, + false, model.StateNone}, + // Test unique index fail to commit, this case needs the new index could be inserted + {[]string{"insert into t1 values(3, 30, 300)", + "insert into t1 values(4, 40, 300)", + "insert into t2 values(11, 11, 11)", + "insert into t2 values(12, 11, 12)"}, + //[]string{"alter table t1 add unique index uk3(c3)", "alter table t1 drop index uk3"}, + []string{}, + "alter table t1 add unique index uk3(c3)", + []string{"select c3, c2 from t1 use index(uk3) where c3 = 200", + "select c3, c2 from t1 use index(uk3) where c3 = 300", + "select c3, c2 from t1 where c1 = 4", + "select * from t1", + "select * from t2"}, + [][]string{{"200 20"}, + {}, + {}, + {"1 10 100", "2 20 200"}, + {"1 10 100", "2 20 200"}}, + true, + model.StateWriteOnly}, + */ + } + tk.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) + + // Test add index state change + do := s.dom.DDL() + startStates := []model.SchemaState{model.StateNone, model.StateDeleteOnly} + for _, startState := range startStates { + endStatMap := session.ConstOpAddIndex[startState] + var endStates []model.SchemaState + for st := range endStatMap { + endStates = append(endStates, st) + } + sort.Slice(endStates, func(i, j int) bool { return endStates[i] < endStates[j] }) + for _, endState := range endStates { + for _, curCase := range cases { + if endState < curCase.stateEnd { + break + } + tk2.MustExec("drop table if exists t1") + tk2.MustExec("drop table if exists t2") + tk2.MustExec("create table t1 (c1 int primary key, c2 int, c3 int, index ok2(c2))") + tk2.MustExec("create table t2 (c1 int primary key, c2 int, c3 int, index ok2(c2))") + tk2.MustExec("insert t1 values (1, 10, 100), (2, 20, 200)") + tk2.MustExec("insert t2 values (1, 10, 100), (2, 20, 200)") + tk2.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) + tk.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) + tk.MustQuery("select * from t2;").Check(testkit.Rows("1 10 100", "2 20 200")) + + for _, DDLSQL := range curCase.tk2DDL { + tk2.MustExec(DDLSQL) + } + hook := &ddl.TestDDLCallback{} + prepared := false + committed := false + hook.OnJobUpdatedExported = func(job *model.Job) { + if job.SchemaState == startState { + if !prepared { + tk.MustExec("begin pessimistic") + for _, tkSQL := range curCase.tkSQLs { + tk.MustExec(tkSQL) + } + prepared = true + } + } else if job.SchemaState == endState { + if !committed { + if curCase.failCommit { + _, err := tk.Exec("commit") + c.Assert(err, NotNil) + } else { + tk.MustExec("commit") + } + } + committed = true + } + } + originalCallback := do.GetHook() + do.(ddl.DDLForTest).SetHook(hook) + tk2.MustExec(curCase.idxDDL) + do.(ddl.DDLForTest).SetHook(originalCallback) + tk2.MustExec("admin check table t1") + for i, checkSQL := range curCase.checkSQLs { + if len(curCase.rowsExps[i]) > 0 { + tk2.MustQuery(checkSQL).Check(testkit.Rows(curCase.rowsExps[i]...)) + } else { + tk2.MustQuery(checkSQL).Check(nil) + } + } + } + } + } + tk.MustExec("admin check table t1") +} + +// TestAddIndexFailOnCaseWhenCanExit is used to close #19325. +func (s *testSerialDBSuite) TestAddIndexFailOnCaseWhenCanExit(c *C) { + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/MockCaseWhenParseFailure", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/MockCaseWhenParseFailure"), IsNil) + }() + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a int, b int)") + tk.MustExec("insert into t values(1, 1)") + _, err := tk.Exec("alter table t add index idx(b)") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[ddl:-1]DDL job rollback, error msg: job.ErrCount:512, mock unknown type: ast.whenClause.") + tk.MustExec("drop table if exists t") +} + +>>>>>>> 1739f2a... ddl: exit add index on generated column with `case-when` expression parse error (#19330) func init() { // Make sure it will only be executed once. domain.SchemaOutOfDateRetryInterval = int64(50 * time.Millisecond) diff --git a/ddl/index.go b/ddl/index.go index 334aa60284102..a43098c5fa5d6 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -1361,6 +1361,16 @@ func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, indexInfo *model.I logutil.BgLogger().Info("[ddl] start to add table index", zap.String("job", job.String()), zap.String("reorgInfo", reorgInfo.String())) totalAddedCount := job.GetRowCount() + if err := w.isReorgRunnable(reorgInfo.d); err != nil { + return errors.Trace(err) + } + + failpoint.Inject("MockCaseWhenParseFailure", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(errors.New("job.ErrCount:" + strconv.Itoa(int(job.ErrorCount)) + ", mock unknown type: ast.whenClause.")) + } + }) + startHandle, endHandle := reorgInfo.StartHandle, reorgInfo.EndHandle sessCtx := newContext(reorgInfo.d.store) decodeColMap, err := makeupDecodeColMap(sessCtx, t, indexInfo) @@ -1368,6 +1378,13 @@ func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, indexInfo *model.I return errors.Trace(err) } +<<<<<<< HEAD +======= + if startHandle == nil && endHandle == nil { + return nil + } + +>>>>>>> 1739f2a... ddl: exit add index on generated column with `case-when` expression parse error (#19330) // variable.ddlReorgWorkerCounter can be modified by system variable "tidb_ddl_reorg_worker_cnt". workerCnt := variable.GetDDLReorgWorkerCounter() idxWorkers := make([]*addIndexWorker, 0, workerCnt) From d2c082ecdeb87f0459938e2b4b79ee1d4aac9a4a Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Mon, 24 Aug 2020 18:00:17 +0800 Subject: [PATCH 2/2] . Signed-off-by: AilinKid <314806019@qq.com> --- ddl/db_test.go | 216 +------------------------------------------------ ddl/index.go | 7 -- 2 files changed, 1 insertion(+), 222 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 425bdaecb19ca..4c3cea1e73624 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -4676,219 +4676,6 @@ func (s *testSerialDBSuite) TestDDLJobErrorCount(c *C) { } } -<<<<<<< HEAD -======= -func (s *testDBSuite1) TestAlterTableWithValidation(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t1") - defer tk.MustExec("drop table if exists t1") - - tk.MustExec("create table t1 (c1 int, c2 int as (c1 + 1));") - - // Test for alter table with validation. - tk.MustExec("alter table t1 with validation") - c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(1)) - tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|8200|ALTER TABLE WITH VALIDATION is currently unsupported")) - - // Test for alter table without validation. - tk.MustExec("alter table t1 without validation") - c.Assert(tk.Se.GetSessionVars().StmtCtx.WarningCount(), Equals, uint16(1)) - tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|8200|ALTER TABLE WITHOUT VALIDATION is currently unsupported")) -} - -func (s *testSerialDBSuite) TestCommitTxnWithIndexChange(c *C) { - // Prepare work. - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("drop database if exists test_db") - tk.MustExec("create database test_db") - tk.MustExec("use test_db") - tk.MustExec("create table t1 (c1 int primary key, c2 int, c3 int, index ok2(c2))") - tk.MustExec("insert t1 values (1, 10, 100), (2, 20, 200)") - tk.MustExec("alter table t1 add index k2(c2)") - tk.MustExec("alter table t1 drop index k2") - tk.MustExec("alter table t1 add index k2(c2)") - tk.MustExec("alter table t1 drop index k2") - tk2 := testkit.NewTestKit(c, s.store) - tk2.MustExec("use test_db") - - // tkSQLs are the sql statements for the pessimistic transaction. - // tk2DDL are the ddl statements executed before the pessimistic transaction. - // idxDDL is the DDL statement executed between pessimistic transaction begin and commit. - // failCommit means the pessimistic transaction commit should fail not. - type caseUnit struct { - tkSQLs []string - tk2DDL []string - idxDDL string - checkSQLs []string - rowsExps [][]string - failCommit bool - stateEnd model.SchemaState - } - - cases := []caseUnit{ - // Test secondary index - {[]string{"insert into t1 values(3, 30, 300)", - "insert into t2 values(11, 11, 11)"}, - []string{"alter table t1 add index k2(c2)", - "alter table t1 drop index k2", - "alter table t1 add index kk2(c2, c1)", - "alter table t1 add index k2(c2)", - "alter table t1 drop index k2"}, - "alter table t1 add index k2(c2)", - []string{"select c3, c2 from t1 use index(k2) where c2 = 20", - "select c3, c2 from t1 use index(k2) where c2 = 10", - "select * from t1", - "select * from t2 where c1 = 11"}, - [][]string{{"200 20"}, - {"100 10"}, - {"1 10 100", "2 20 200", "3 30 300"}, - {"11 11 11"}}, - false, - model.StateNone}, - // Test secondary index - {[]string{"insert into t2 values(5, 50, 500)", - "insert into t2 values(11, 11, 11)", - "delete from t2 where c2 = 11", - "update t2 set c2 = 110 where c1 = 11"}, - //"update t2 set c1 = 10 where c3 = 100"}, - []string{"alter table t1 add index k2(c2)", - "alter table t1 drop index k2", - "alter table t1 add index kk2(c2, c1)", - "alter table t1 add index k2(c2)", - "alter table t1 drop index k2"}, - "alter table t1 add index k2(c2)", - []string{"select c3, c2 from t1 use index(k2) where c2 = 20", - "select c3, c2 from t1 use index(k2) where c2 = 10", - "select * from t1", - "select * from t2 where c1 = 11", - "select * from t2 where c3 = 100"}, - [][]string{{"200 20"}, - {"100 10"}, - {"1 10 100", "2 20 200"}, - {}, - {"1 10 100"}}, - false, - model.StateNone}, - // Test unique index - /* TODO unique index is not supported now. - {[]string{"insert into t1 values(3, 30, 300)", - "insert into t1 values(4, 40, 400)", - "insert into t2 values(11, 11, 11)", - "insert into t2 values(12, 12, 11)"}, - []string{"alter table t1 add unique index uk3(c3)", - "alter table t1 drop index uk3", - "alter table t2 add unique index ukc1c3(c1, c3)", - "alter table t2 add unique index ukc3(c3)", - "alter table t2 drop index ukc1c3", - "alter table t2 drop index ukc3", - "alter table t2 add index kc3(c3)"}, - "alter table t1 add unique index uk3(c3)", - []string{"select c3, c2 from t1 use index(uk3) where c3 = 200", - "select c3, c2 from t1 use index(uk3) where c3 = 300", - "select c3, c2 from t1 use index(uk3) where c3 = 400", - "select * from t1", - "select * from t2"}, - [][]string{{"200 20"}, - {"300 30"}, - {"400 40"}, - {"1 10 100", "2 20 200", "3 30 300", "4 40 400"}, - {"1 10 100", "2 20 200", "11 11 11", "12 12 11"}}, - false, model.StateNone}, - // Test unique index fail to commit, this case needs the new index could be inserted - {[]string{"insert into t1 values(3, 30, 300)", - "insert into t1 values(4, 40, 300)", - "insert into t2 values(11, 11, 11)", - "insert into t2 values(12, 11, 12)"}, - //[]string{"alter table t1 add unique index uk3(c3)", "alter table t1 drop index uk3"}, - []string{}, - "alter table t1 add unique index uk3(c3)", - []string{"select c3, c2 from t1 use index(uk3) where c3 = 200", - "select c3, c2 from t1 use index(uk3) where c3 = 300", - "select c3, c2 from t1 where c1 = 4", - "select * from t1", - "select * from t2"}, - [][]string{{"200 20"}, - {}, - {}, - {"1 10 100", "2 20 200"}, - {"1 10 100", "2 20 200"}}, - true, - model.StateWriteOnly}, - */ - } - tk.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) - - // Test add index state change - do := s.dom.DDL() - startStates := []model.SchemaState{model.StateNone, model.StateDeleteOnly} - for _, startState := range startStates { - endStatMap := session.ConstOpAddIndex[startState] - var endStates []model.SchemaState - for st := range endStatMap { - endStates = append(endStates, st) - } - sort.Slice(endStates, func(i, j int) bool { return endStates[i] < endStates[j] }) - for _, endState := range endStates { - for _, curCase := range cases { - if endState < curCase.stateEnd { - break - } - tk2.MustExec("drop table if exists t1") - tk2.MustExec("drop table if exists t2") - tk2.MustExec("create table t1 (c1 int primary key, c2 int, c3 int, index ok2(c2))") - tk2.MustExec("create table t2 (c1 int primary key, c2 int, c3 int, index ok2(c2))") - tk2.MustExec("insert t1 values (1, 10, 100), (2, 20, 200)") - tk2.MustExec("insert t2 values (1, 10, 100), (2, 20, 200)") - tk2.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) - tk.MustQuery("select * from t1;").Check(testkit.Rows("1 10 100", "2 20 200")) - tk.MustQuery("select * from t2;").Check(testkit.Rows("1 10 100", "2 20 200")) - - for _, DDLSQL := range curCase.tk2DDL { - tk2.MustExec(DDLSQL) - } - hook := &ddl.TestDDLCallback{} - prepared := false - committed := false - hook.OnJobUpdatedExported = func(job *model.Job) { - if job.SchemaState == startState { - if !prepared { - tk.MustExec("begin pessimistic") - for _, tkSQL := range curCase.tkSQLs { - tk.MustExec(tkSQL) - } - prepared = true - } - } else if job.SchemaState == endState { - if !committed { - if curCase.failCommit { - _, err := tk.Exec("commit") - c.Assert(err, NotNil) - } else { - tk.MustExec("commit") - } - } - committed = true - } - } - originalCallback := do.GetHook() - do.(ddl.DDLForTest).SetHook(hook) - tk2.MustExec(curCase.idxDDL) - do.(ddl.DDLForTest).SetHook(originalCallback) - tk2.MustExec("admin check table t1") - for i, checkSQL := range curCase.checkSQLs { - if len(curCase.rowsExps[i]) > 0 { - tk2.MustQuery(checkSQL).Check(testkit.Rows(curCase.rowsExps[i]...)) - } else { - tk2.MustQuery(checkSQL).Check(nil) - } - } - } - } - } - tk.MustExec("admin check table t1") -} - // TestAddIndexFailOnCaseWhenCanExit is used to close #19325. func (s *testSerialDBSuite) TestAddIndexFailOnCaseWhenCanExit(c *C) { c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/MockCaseWhenParseFailure", `return(true)`), IsNil) @@ -4902,11 +4689,10 @@ func (s *testSerialDBSuite) TestAddIndexFailOnCaseWhenCanExit(c *C) { tk.MustExec("insert into t values(1, 1)") _, err := tk.Exec("alter table t add index idx(b)") c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "[ddl:-1]DDL job rollback, error msg: job.ErrCount:512, mock unknown type: ast.whenClause.") + c.Assert(err.Error(), Equals, "[ddl:8214]Cancelled DDL job") tk.MustExec("drop table if exists t") } ->>>>>>> 1739f2a... ddl: exit add index on generated column with `case-when` expression parse error (#19330) func init() { // Make sure it will only be executed once. domain.SchemaOutOfDateRetryInterval = int64(50 * time.Millisecond) diff --git a/ddl/index.go b/ddl/index.go index a43098c5fa5d6..1e9daf418e6c5 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -1378,13 +1378,6 @@ func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, indexInfo *model.I return errors.Trace(err) } -<<<<<<< HEAD -======= - if startHandle == nil && endHandle == nil { - return nil - } - ->>>>>>> 1739f2a... ddl: exit add index on generated column with `case-when` expression parse error (#19330) // variable.ddlReorgWorkerCounter can be modified by system variable "tidb_ddl_reorg_worker_cnt". workerCnt := variable.GetDDLReorgWorkerCounter() idxWorkers := make([]*addIndexWorker, 0, workerCnt)