From 1162a3fba7c25f60fe73e10ec00a5ae3627b4e50 Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 24 Oct 2018 21:09:27 +0800 Subject: [PATCH 1/2] ddl, executor: fix the issue of executing DDL after executing SQL failure in txn --- ddl/ddl.go | 5 ----- ddl/foreign_key_test.go | 4 +++- executor/ddl.go | 5 +++++ executor/ddl_test.go | 17 +++++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ddl/ddl.go b/ddl/ddl.go index 6114e51950142..f9c76425685ca 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -478,11 +478,6 @@ func (d *ddl) asyncNotifyWorker(jobTp model.ActionType) { } func (d *ddl) doDDLJob(ctx sessionctx.Context, job *model.Job) error { - // For every DDL, we must commit current transaction. - if err := ctx.NewTxn(); err != nil { - return errors.Trace(err) - } - // Get a global job ID and put the DDL job in the queue. err := d.addDDLJob(ctx, job) if err != nil { diff --git a/ddl/foreign_key_test.go b/ddl/foreign_key_test.go index 7fdde31834477..75f5700a6b614 100644 --- a/ddl/foreign_key_test.go +++ b/ddl/foreign_key_test.go @@ -78,7 +78,9 @@ func (s *testForeighKeySuite) testCreateForeignKey(c *C, tblInfo *model.TableInf BinlogInfo: &model.HistoryInfo{}, Args: []interface{}{fkInfo}, } - err := s.d.doDDLJob(s.ctx, job) + err := s.ctx.NewTxn() + c.Assert(err, IsNil) + err = s.d.doDDLJob(s.ctx, job) c.Assert(err, IsNil) return job } diff --git a/executor/ddl.go b/executor/ddl.go index d972c69a7b18e..217b1cac344b6 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -67,6 +67,11 @@ func (e *DDLExec) Next(ctx context.Context, chk *chunk.Chunk) (err error) { return nil } e.done = true + + // For every DDL, we must commit current transaction. + if err := e.ctx.NewTxn(); err != nil { + return errors.Trace(err) + } defer func() { e.ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue = false }() switch x := e.stmt.(type) { diff --git a/executor/ddl_test.go b/executor/ddl_test.go index b91a57b4b1559..45884d1c01e64 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -46,6 +46,23 @@ func (s *testSuite) TestTruncateTable(c *C) { result.Check(nil) } +// TestInTxnExecDDLFail tests the following case: +// 1. Execute the SQL of "begin"; +// 2. A SQL that will fail to execute; +// 3. Execute DDL. +func (s *testSuite) TestInTxnExecDDLFail(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create table t (i int key);") + tk.MustExec("insert into t values (1);") + tk.MustExec("begin;") + tk.MustExec("insert into t values (1);") + _, err := tk.Exec("truncate table t;") + c.Assert(err.Error(), Equals, "[kv:1062]Duplicate entry '1' for key 'PRIMARY'") + result := tk.MustQuery("select count(*) from t") + result.Check(testkit.Rows("1")) +} + func (s *testSuite) TestCreateTable(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") From 361a41df68e7dd5dcdd7465d1333d71fb930c95f Mon Sep 17 00:00:00 2001 From: Lynn Date: Thu, 25 Oct 2018 13:32:01 +0800 Subject: [PATCH 2/2] executor: address comments --- executor/ddl.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/ddl.go b/executor/ddl.go index 217b1cac344b6..34b4744385083 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -68,8 +68,8 @@ func (e *DDLExec) Next(ctx context.Context, chk *chunk.Chunk) (err error) { } e.done = true - // For every DDL, we must commit current transaction. - if err := e.ctx.NewTxn(); err != nil { + // For each DDL, we should commit the previous transaction and create a new transaction. + if err = e.ctx.NewTxn(); err != nil { return errors.Trace(err) } defer func() { e.ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue = false }()