Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner: fix wrong PointGet / TableDual plan reused in plan cache (#23238) #24043

Merged
merged 2 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions executor/explainfor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,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 @@ -255,7 +255,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 @@ -267,7 +266,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 @@ -410,6 +410,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.IsFlashProp() {
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 @@ -1053,3 +1053,96 @@ func (s *testPlanSerialSuite) TestIssue23671(c *C) {
tk.MustQuery("execute s1 using @a, @b, @c").Check(testkit.Rows("1 1", "2 2"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

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