From 661eae470f3faf1575910ac4c5aaac8ec32861e1 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Tue, 13 Aug 2019 12:16:44 +0800 Subject: [PATCH 1/4] executor: set the correct stmtCtx for explain statement --- executor/executor.go | 11 +++++++---- executor/explainfor_test.go | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/executor/executor.go b/executor/executor.go index 01a28bf0cde4b..24d9b37c07a68 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -1280,12 +1280,19 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) { sc.MemTracker.SetActionOnExceed(action) } + // execute missed stmtID uses empty sql + sc.OriginalSQL = s.Text() + if execStmt, ok := s.(*ast.ExecuteStmt); ok { s, err = getPreparedStmt(execStmt, vars) if err != nil { return } } + if explainStmt, ok := s.(*ast.ExplainStmt); ok { + sc.InExplainStmt = true + s = explainStmt.Stmt + } // TODO: Many same bool variables here. // We should set only two variables ( // IgnoreErr and StrictSQLMode) to avoid setting the same bool variables and @@ -1346,8 +1353,6 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) { sc.NotFillCache = !opts.SQLCache } sc.PadCharToFullLength = ctx.GetSessionVars().SQLMode.HasPadCharToFullLengthMode() - case *ast.ExplainStmt: - sc.InExplainStmt = true case *ast.ShowStmt: sc.IgnoreTruncate = true sc.IgnoreZeroInDate = true @@ -1391,8 +1396,6 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) { if err != nil { return err } - // execute missed stmtID uses empty sql - sc.OriginalSQL = s.Text() vars.StmtCtx = sc return } diff --git a/executor/explainfor_test.go b/executor/explainfor_test.go index b1ef27ea1bd19..632874180495b 100644 --- a/executor/explainfor_test.go +++ b/executor/explainfor_test.go @@ -79,3 +79,23 @@ func (s *testSuite) TestExplainFor(c *C) { tkRoot.Se.SetSessionManager(&mockSessionManager1{PS: ps}) tkRoot.MustExec(fmt.Sprintf("explain for connection %d", tkRootProcess.ID)) } + +func (s *testSuite) TestIssue11124(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk2 := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("create table kankan1(id int, name text);") + tk.MustExec("create table kankan2(id int, h1 text);") + tk.MustExec("insert into kankan1 values(1, 'a'), (2, 'a');") + tk.MustExec("insert into kankan2 values(2, 'z');") + tk.MustQuery("select t1.id from kankan1 t1 left join kankan2 t2 on t1.id = t2.id where (case when t1.name='b' then 'case2' when t1.name='a' then 'case1' else NULL end) = 'case1'") + tkRootProcess := tk.Se.ShowProcess() + ps := []*util.ProcessInfo{tkRootProcess} + tk.Se.SetSessionManager(&mockSessionManager1{PS: ps}) + tk2.Se.SetSessionManager(&mockSessionManager1{PS: ps}) + + rs := tk.MustQuery("explain select t1.id from kankan1 t1 left join kankan2 t2 on t1.id = t2.id where (case when t1.name='b' then 'case2' when t1.name='a' then 'case1' else NULL end) = 'case1'").Rows() + rs2 := tk2.MustQuery(fmt.Sprintf("explain for connection %d", tkRootProcess.ID)).Rows() + for i := range rs { + c.Assert(rs[i], DeepEquals, rs2[i]) + } +} From f303a732c72cc8b90970cb26e6bcb0aea9c1a65b Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Mon, 26 Aug 2019 16:30:38 +0800 Subject: [PATCH 2/4] test --- executor/point_get_test.go | 3 +++ util/testkit/testkit.go | 2 ++ 2 files changed, 5 insertions(+) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 25fcee26c8ad7..cad4f2936ff2d 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -23,6 +23,7 @@ import ( "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/util/testkit" + "github.com/sirupsen/logrus" ) type testPointGetSuite struct { @@ -272,7 +273,9 @@ func (s *testPointGetSuite) TestIndexLookupChar(c *C) { // Test truncate with sql mode `PAD_CHAR_TO_FULL_LENGTH`. tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) tk.MustIndexLookup(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) + logrus.Warning("============================") tk.MustIndexLookup(`select * from t where a = "aab";`).Check(testkit.Rows()) + logrus.Warning("============================") tk.MustExec(`truncate table t;`) tk.MustExec(`insert into t values("a ", "b ");`) diff --git a/util/testkit/testkit.go b/util/testkit/testkit.go index 947afe70961de..821d3b8a93fe8 100644 --- a/util/testkit/testkit.go +++ b/util/testkit/testkit.go @@ -32,6 +32,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/sqlexec" "github.com/pingcap/tidb/util/testutil" + "github.com/sirupsen/logrus" ) // TestKit is a utility to run sql test. @@ -190,6 +191,7 @@ func (tk *TestKit) MustIndexLookup(sql string, args ...interface{}) *Result { rs := tk.MustQuery("explain "+sql, args...) hasIndexLookup := false for i := range rs.rows { + logrus.Warning(rs.rows[i][0]) if strings.Contains(rs.rows[i][0], "IndexLookUp") { hasIndexLookup = true break From 710028fb9964da2c055231aa0cc89470f63e0269 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Mon, 26 Aug 2019 21:32:26 +0800 Subject: [PATCH 3/4] fix ci --- executor/point_get_test.go | 9 +++------ util/testkit/testkit.go | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index cad4f2936ff2d..d888c7417513f 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -23,7 +23,6 @@ import ( "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/tikv" "github.com/pingcap/tidb/util/testkit" - "github.com/sirupsen/logrus" ) type testPointGetSuite struct { @@ -273,9 +272,7 @@ func (s *testPointGetSuite) TestIndexLookupChar(c *C) { // Test truncate with sql mode `PAD_CHAR_TO_FULL_LENGTH`. tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) tk.MustIndexLookup(`select * from t where a = "aa";`).Check(testkit.Rows(`aa bb`)) - logrus.Warning("============================") - tk.MustIndexLookup(`select * from t where a = "aab";`).Check(testkit.Rows()) - logrus.Warning("============================") + tk.MustTableDual(`select * from t where a = "aab";`).Check(testkit.Rows()) tk.MustExec(`truncate table t;`) tk.MustExec(`insert into t values("a ", "b ");`) @@ -288,9 +285,9 @@ func (s *testPointGetSuite) TestIndexLookupChar(c *C) { // Test trailing spaces with sql mode `PAD_CHAR_TO_FULL_LENGTH`. tk.MustExec(`set @@sql_mode="PAD_CHAR_TO_FULL_LENGTH";`) - tk.MustIndexLookup(`select * from t where a = "a";`).Check(testkit.Rows()) + tk.MustTableDual(`select * from t where a = "a";`).Check(testkit.Rows()) tk.MustIndexLookup(`select * from t where a = "a ";`).Check(testkit.Rows(`a b`)) - tk.MustIndexLookup(`select * from t where a = "a ";`).Check(testkit.Rows()) + tk.MustTableDual(`select * from t where a = "a ";`).Check(testkit.Rows()) // Test CHAR BINARY. tk.MustExec(`drop table if exists t;`) diff --git a/util/testkit/testkit.go b/util/testkit/testkit.go index 821d3b8a93fe8..8caa015e81861 100644 --- a/util/testkit/testkit.go +++ b/util/testkit/testkit.go @@ -32,7 +32,6 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/sqlexec" "github.com/pingcap/tidb/util/testutil" - "github.com/sirupsen/logrus" ) // TestKit is a utility to run sql test. @@ -191,7 +190,6 @@ func (tk *TestKit) MustIndexLookup(sql string, args ...interface{}) *Result { rs := tk.MustQuery("explain "+sql, args...) hasIndexLookup := false for i := range rs.rows { - logrus.Warning(rs.rows[i][0]) if strings.Contains(rs.rows[i][0], "IndexLookUp") { hasIndexLookup = true break @@ -201,6 +199,20 @@ func (tk *TestKit) MustIndexLookup(sql string, args ...interface{}) *Result { return tk.MustQuery(sql, args...) } +// MustTableDual checks whether the plan for the sql is TableDual. +func (tk *TestKit) MustTableDual(sql string, args ...interface{}) *Result { + rs := tk.MustQuery("explain "+sql, args...) + hasTableDual := false + for i := range rs.rows { + if strings.Contains(rs.rows[i][0], "TableDual") { + hasTableDual = true + break + } + } + tk.c.Assert(hasTableDual, check.IsTrue) + return tk.MustQuery(sql, args...) +} + // MustPointGet checks whether the plan for the sql is Point_Get. func (tk *TestKit) MustPointGet(sql string, args ...interface{}) *Result { rs := tk.MustQuery("explain "+sql, args...) From bc766869ed070fd8792055c5073a52d5880a2813 Mon Sep 17 00:00:00 2001 From: xuhuaiyu <391585975@qq.com> Date: Tue, 27 Aug 2019 12:10:12 +0800 Subject: [PATCH 4/4] address comment --- executor/executor.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/executor/executor.go b/executor/executor.go index df253b2450e4c..2b258f5669cba 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -1386,16 +1386,14 @@ func ResetContextOfStmt(ctx sessionctx.Context, s ast.StmtNode) (err error) { action.SetLogHook(domain.GetDomain(ctx).ExpensiveQueryHandle().LogOnQueryExceedMemQuota) sc.MemTracker.SetActionOnExceed(action) } - - // execute missed stmtID uses empty sql - sc.OriginalSQL = s.Text() - if execStmt, ok := s.(*ast.ExecuteStmt); ok { s, err = getPreparedStmt(execStmt, vars) if err != nil { return } } + // execute missed stmtID uses empty sql + sc.OriginalSQL = s.Text() if explainStmt, ok := s.(*ast.ExplainStmt); ok { sc.InExplainStmt = true sc.CastStrToIntStrict = true