Skip to content

Commit

Permalink
planner: fix wrong PointGet / TableDual plan reused in plan cache (#2…
Browse files Browse the repository at this point in the history
  • Loading branch information
eurekaka authored Mar 17, 2021
1 parent bb12864 commit 1ac53c5
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 16 deletions.
3 changes: 0 additions & 3 deletions executor/explainfor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,4 @@ func (s *testPrepareSerialSuite) TestPointGetUserVarPlanCache(c *C) {
tk.MustQuery("execute stmt using @a").Check(testkit.Rows(
"2 4 2 2",
))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows(
"1",
))
}
2 changes: 0 additions & 2 deletions executor/prepared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ func (s *testSerialSuite) TestPlanCacheClusterIndex(c *C) {
tk.MustExec(`set @v1 = 'a', @v2 = 'b'`)
tk.MustQuery(`execute stmt1 using @v1`).Check(testkit.Rows("a 1 a 1"))
tk.MustQuery(`execute stmt1 using @v2`).Check(testkit.Rows("b 2 b 2"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))

// case 2:
tk.MustExec(`drop table if exists ta, tb`)
Expand All @@ -266,7 +265,6 @@ func (s *testSerialSuite) TestPlanCacheClusterIndex(c *C) {
tk.MustExec(`set @v1 = 'a', @v2 = 'b'`)
tk.MustQuery(`execute stmt1 using @v1`).Check(testkit.Rows("a 1 1 1"))
tk.MustQuery(`execute stmt1 using @v2`).Check(testkit.Rows("b 2 2 2"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))
tk.MustQuery(`execute stmt1 using @v2`).Check(testkit.Rows("b 2 2 2"))
tkProcess = tk.Se.ShowProcess()
ps = []*util.ProcessInfo{tkProcess}
Expand Down
4 changes: 2 additions & 2 deletions executor/seqtest/prepared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,14 +490,14 @@ func (s *seqTestSuite) TestPreparedInsert(c *C) {
err = counter.Write(pb)
c.Assert(err, IsNil)
hit := pb.GetCounter().GetValue()
c.Check(hit, Equals, float64(3))
c.Check(hit, Equals, float64(2))
}
tk.MustExec(`set @a=3; execute stmt_insert_select using @a;`)
if flag {
err = counter.Write(pb)
c.Assert(err, IsNil)
hit := pb.GetCounter().GetValue()
c.Check(hit, Equals, float64(4))
c.Check(hit, Equals, float64(2))
}

result = tk.MustQuery("select id, c1 from prepare_test where id = ?", 101)
Expand Down
3 changes: 3 additions & 0 deletions planner/core/common_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ REBUILD:
// short paths for these executions, currently "point select" and "point update"
func (e *Execute) tryCachePointPlan(ctx context.Context, sctx sessionctx.Context,
preparedStmt *CachedPrepareStmt, is infoschema.InfoSchema, p Plan) error {
if sctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst {
return nil
}
var (
prepared = preparedStmt.PreparedAst
ok bool
Expand Down
2 changes: 1 addition & 1 deletion planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ func (ds *DataSource) skylinePruning(prop *property.PhysicalProperty) []*candida
continue
}
// if we already know the range of the scan is empty, just return a TableDual
if len(path.Ranges) == 0 && !ds.ctx.GetSessionVars().StmtCtx.UseCache {
if len(path.Ranges) == 0 {
return []*candidatePath{{path: path}}
}
if path.StoreType != kv.TiFlash && (prop.TaskTp == property.CopTiFlashLocalReadTaskType || prop.TaskTp == property.CopTiFlashGlobalReadTaskType) {
Expand Down
93 changes: 93 additions & 0 deletions planner/core/prepare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1036,3 +1036,96 @@ func (s *testPlanSerialSuite) TestPlanCacheSnapshot(c *C) {
tk.MustQuery("execute stmt using @p").Check(testkit.Rows("1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))
}

func (s *testPlanSerialSuite) TestPlanCachePointGetAndTableDual(c *C) {
store, _, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
tk := testkit.NewTestKit(c, store)
orgEnable := core.PreparedPlanCacheEnabled()
defer func() {
store.Close()
core.SetPreparedPlanCache(orgEnable)
}()
core.SetPreparedPlanCache(true)

tk.Se, err = session.CreateSession4TestWithOpt(store, &session.Opt{
PreparedPlanCache: kvcache.NewSimpleLRUCache(100, 0.1, math.MaxUint64),
})
c.Assert(err, IsNil)

tk.MustExec("use test")
tk.MustExec("drop table if exists t0, t1, t2, t3, t4")

tk.MustExec("create table t0(c1 varchar(20), c2 varchar(20), c3 bigint(20), primary key(c1, c2))")
tk.MustExec("insert into t0 values('0000','7777',1)")
tk.MustExec("prepare s0 from 'select * from t0 where c1=? and c2>=? and c2<=?'")
tk.MustExec("set @a0='0000', @b0='9999'")
// TableDual plan would be built, we should not cache it.
tk.MustQuery("execute s0 using @a0, @b0, @a0").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous TableDual plan.
tk.MustQuery("execute s0 using @a0, @a0, @b0").Check(testkit.Rows("0000 7777 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("create table t1(c1 varchar(20), c2 varchar(20), c3 bigint(20), primary key(c1, c2))")
tk.MustExec("insert into t1 values('0000','7777',1)")
tk.MustExec("prepare s1 from 'select * from t1 where c1=? and c2>=? and c2<=?'")
tk.MustExec("set @a1='0000', @b1='9999'")
// PointGet plan would be built, we should not cache it.
tk.MustQuery("execute s1 using @a1, @b1, @b1").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous PointGet plan.
tk.MustQuery("execute s1 using @a1, @a1, @b1").Check(testkit.Rows("0000 7777 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("create table t2(c1 bigint(20) primary key, c2 varchar(20))")
tk.MustExec("insert into t2 values(1,'7777')")
tk.MustExec("prepare s2 from 'select * from t2 where c1>=? and c1<=?'")
tk.MustExec("set @a2=0, @b2=9")
// PointGet plan would be built, we should not cache it.
tk.MustQuery("execute s2 using @a2, @a2").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous PointGet plan.
tk.MustQuery("execute s2 using @a2, @b2").Check(testkit.Rows("1 7777"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("create table t3(c1 int, c2 int, c3 int, unique key(c1), key(c2))")
tk.MustExec("insert into t3 values(2,1,1)")
tk.MustExec("prepare s3 from 'select /*+ use_index_merge(t3) */ * from t3 where (c1 >= ? and c1 <= ?) or c2 > 1'")
tk.MustExec("set @a3=1,@b3=3")
// PointGet partial plan would be built, we should not cache it.
tk.MustQuery("execute s3 using @a3,@a3").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous IndexMerge with partial PointGet plan.
tk.MustQuery("execute s3 using @a3,@b3").Check(testkit.Rows("2 1 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("prepare s3 from 'select /*+ use_index_merge(t3) */ * from t3 where (c1 >= ? and c1 <= ?) or c2 > 1'")
tk.MustExec("set @a3=1,@b3=3")
// TableDual partial plan would be built, we should not cache it.
tk.MustQuery("execute s3 using @b3,@a3").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous IndexMerge with partial TableDual plan.
tk.MustQuery("execute s3 using @a3,@b3").Check(testkit.Rows("2 1 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("create table t4(c1 int primary key, c2 int, c3 int, key(c2))")
tk.MustExec("insert into t4 values(2,1,1)")
tk.MustExec("prepare s4 from 'select /*+ use_index_merge(t4) */ * from t4 where (c1 >= ? and c1 <= ?) or c2 > 1'")
tk.MustExec("set @a4=1,@b4=3")
// PointGet partial plan would be built, we should not cache it.
tk.MustQuery("execute s4 using @a4,@a4").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous IndexMerge with partial PointGet plan.
tk.MustQuery("execute s4 using @a4,@b4").Check(testkit.Rows("2 1 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))

tk.MustExec("prepare s4 from 'select /*+ use_index_merge(t4) */ * from t4 where (c1 >= ? and c1 <= ?) or c2 > 1'")
tk.MustExec("set @a4=1,@b4=3")
// TableDual partial plan would be built, we should not cache it.
tk.MustQuery("execute s4 using @b4,@a4").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
// Must not reuse the previous IndexMerge with partial TableDual plan.
tk.MustQuery("execute s4 using @a4,@b4").Check(testkit.Rows("2 1 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}
16 changes: 8 additions & 8 deletions planner/core/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ func (ds *DataSource) DeriveStats(childStats []*property.StatsInfo, selfSchema *
if noIntervalRanges || len(path.Ranges) == 0 {
ds.possibleAccessPaths[0] = path
ds.possibleAccessPaths = ds.possibleAccessPaths[:1]
ds.ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true
break
}
continue
Expand All @@ -294,6 +295,7 @@ func (ds *DataSource) DeriveStats(childStats []*property.StatsInfo, selfSchema *
if (noIntervalRanges && path.Index.Unique) || len(path.Ranges) == 0 {
ds.possibleAccessPaths[0] = path
ds.possibleAccessPaths = ds.possibleAccessPaths[:1]
ds.ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true
break
}
}
Expand Down Expand Up @@ -506,10 +508,8 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
logutil.BgLogger().Debug("can not derive statistics of a path", zap.Error(err))
continue
}
// If `AccessConds` is empty, we ignore the access path.
// If the path contains a full range, ignore it also. This can happen when `AccessConds` is constant true, and
// it comes from the result of a subquery, so it is not folded.
if len(path.AccessConds) == 0 || ranger.HasFullRange(path.Ranges) {
// If the path contains a full range, ignore it.
if ranger.HasFullRange(path.Ranges) {
continue
}
// If we have point or empty range, just remove other possible paths.
Expand All @@ -520,6 +520,7 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
results[0] = path
results = results[:1]
}
ds.ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true
break
}
} else {
Expand All @@ -533,10 +534,8 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
continue
}
noIntervalRanges := ds.deriveIndexPathStats(path, conditions, true)
// If `AccessConds` is empty, we ignore the access path.
// If the path contains a full range, ignore it also. This can happen when `AccessConds` is constant true, and
// it comes from the result of a subquery, so it is not folded.
if len(path.AccessConds) == 0 || ranger.HasFullRange(path.Ranges) {
// If the path contains a full range, ignore it.
if ranger.HasFullRange(path.Ranges) {
continue
}
// If we have empty range, or point range on unique index, just remove other possible paths.
Expand All @@ -547,6 +546,7 @@ func (ds *DataSource) accessPathsForConds(conditions []expression.Expression, us
results[0] = path
results = results[:1]
}
ds.ctx.GetSessionVars().StmtCtx.OptimDependOnMutableConst = true
break
}
}
Expand Down

0 comments on commit 1ac53c5

Please sign in to comment.