Skip to content

Commit

Permalink
planner: plan cache supports Batch/PointGet converted from (primary k…
Browse files Browse the repository at this point in the history 
…eys) in ((...), ...) (#44838) (#49380)

close #44830
  • Loading branch information
@qw4990
qw4990 authored Dec 12, 2023
1 parent 266d8f7 commit e091ee5
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 24 deletions.
22 changes: 1 addition & 21 deletions planner/core/find_best_task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter

// Batch/PointGet plans may be over-optimized, like `a>=1(?) and a<=1(?)` --> `a=1` --> PointGet(a=1).
// For safety, prevent these plans from the plan cache here.
if !pointGetTask.invalid() && expression.MaybeOverOptimized4PlanCache(ds.ctx, candidate.path.AccessConds) && !ds.isSafePointGetPlan4PlanCache(candidate.path) {
if !pointGetTask.invalid() && expression.MaybeOverOptimized4PlanCache(ds.ctx, candidate.path.AccessConds) && !isSafePointGetPath4PlanCache(ds.ctx, candidate.path) {
ds.ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.New("Batch/PointGet plans may be over-optimized"))
}

Expand Down Expand Up @@ -1147,26 +1147,6 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
return
}

func (ds *DataSource) isSafePointGetPlan4PlanCache(path *util.AccessPath) bool {
// PointGet might contain some over-optimized assumptions, like `a>=1 and a<=1` --> `a=1`, but
// these assumptions may be broken after parameters change.

// safe scenario 1: each column corresponds to a single EQ, `a=1 and b=2 and c=3` --> `[1, 2, 3]`
if len(path.Ranges) > 0 && path.Ranges[0].Width() == len(path.AccessConds) {
for _, accessCond := range path.AccessConds {
f, ok := accessCond.(*expression.ScalarFunction)
if !ok {
return false
}
if f.FuncName.L != ast.EQ {
return false
}
}
return true
}
return false
}

func (ds *DataSource) convertToIndexMergeScan(prop *property.PhysicalProperty, candidate *candidatePath, _ *physicalOptimizeOp) (task task, err error) {
if prop.IsFlashProp() || prop.TaskTp == property.CopSingleReadTaskType || !prop.IsSortItemEmpty() {
return invalidTask, nil
Expand Down
6 changes: 3 additions & 3 deletions planner/core/plan_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ func rebuildRange(p Plan) error {
if err != nil {
return err
}
if !isSafeRange(x.AccessConditions, ranges, false, nil) {
if len(ranges.Ranges) != 1 || !isSafeRange(x.AccessConditions, ranges, false, nil) {
return errors.New("rebuild to get an unsafe range")
}
for i := range x.IndexValues {
Expand All @@ -470,7 +470,7 @@ func rebuildRange(p Plan) error {
if err != nil {
return err
}
if !isSafeRange(x.AccessConditions, &ranger.DetachRangeResult{
if len(ranges) != 1 || !isSafeRange(x.AccessConditions, &ranger.DetachRangeResult{
Ranges: ranges,
AccessConds: accessConds,
RemainedConds: remainingConds,
Expand Down Expand Up @@ -539,7 +539,7 @@ func rebuildRange(p Plan) error {
if err != nil {
return err
}
if len(ranges) != len(x.Handles) && !isSafeRange(x.AccessConditions, &ranger.DetachRangeResult{
if len(ranges) != len(x.Handles) || !isSafeRange(x.AccessConditions, &ranger.DetachRangeResult{
Ranges: ranges,
AccessConds: accessConds,
RemainedConds: remainingConds,
Expand Down
66 changes: 66 additions & 0 deletions planner/core/plan_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,72 @@ func TestIssue43311(t *testing.T) {
tk.MustQuery(`execute st using @a, @b`).Check(testkit.Rows()) // empty
}

func TestIssue44830(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec(`use test`)
tk.MustExec(`set @@tidb_opt_fix_control = "44830:ON"`)
tk.MustExec(`create table t (a int, primary key(a))`)
tk.MustExec(`create table t1 (a int, b int, primary key(a, b))`) // multiple-column primary key
tk.MustExec(`insert into t values (1), (2), (3)`)
tk.MustExec(`insert into t1 values (1, 1), (2, 2), (3, 3)`)
tk.MustExec(`set @a=1, @b=2, @c=3`)

// single-column primary key cases
tk.MustExec(`prepare st from 'select * from t where 1=1 and a in (?, ?, ?)'`)
tk.MustQuery(`execute st using @a, @b, @c`).Sort().Check(testkit.Rows("1", "2", "3"))
tk.MustQuery(`execute st using @a, @b, @c`).Sort().Check(testkit.Rows("1", "2", "3"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1"))
tk.MustQuery(`execute st using @a, @b, @b`).Sort().Check(testkit.Rows("1", "2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // range length changed
tk.MustQuery(`execute st using @b, @b, @b`).Sort().Check(testkit.Rows("2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // range length changed
tk.MustQuery(`execute st using @a, @b, @c`).Sort().Check(testkit.Rows("1", "2", "3"))
tk.MustQuery(`execute st using @a, @b, @b`).Sort().Check(testkit.Rows("1", "2"))
tk.MustQuery(`execute st using @a, @b, @b`).Sort().Check(testkit.Rows("1", "2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // contain duplicated values in the in-list

// multi-column primary key cases
tk.MustExec(`prepare st from 'select * from t1 where 1=1 and (a, b) in ((?, ?), (?, ?), (?, ?))'`)
tk.MustQuery(`execute st using @a, @a, @b, @b, @c, @c`).Sort().Check(testkit.Rows("1 1", "2 2", "3 3"))
tk.MustQuery(`execute st using @a, @a, @b, @b, @c, @c`).Sort().Check(testkit.Rows("1 1", "2 2", "3 3"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1"))
tk.MustQuery(`execute st using @a, @a, @b, @b, @b, @b`).Sort().Check(testkit.Rows("1 1", "2 2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // range length changed
tk.MustQuery(`execute st using @b, @b, @b, @b, @b, @b`).Sort().Check(testkit.Rows("2 2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // range length changed
tk.MustQuery(`execute st using @b, @b, @b, @b, @c, @c`).Sort().Check(testkit.Rows("2 2", "3 3"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // range length changed
tk.MustQuery(`execute st using @a, @a, @a, @a, @a, @a`).Sort().Check(testkit.Rows("1 1"))
tk.MustQuery(`execute st using @a, @a, @a, @a, @a, @a`).Sort().Check(testkit.Rows("1 1"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // contain duplicated values in the in-list
tk.MustQuery(`execute st using @a, @a, @b, @b, @b, @b`).Sort().Check(testkit.Rows("1 1", "2 2"))
tk.MustQuery(`execute st using @a, @a, @b, @b, @b, @b`).Sort().Check(testkit.Rows("1 1", "2 2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0")) // contain duplicated values in the in-list
}

func TestIssue44830NonPrep(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec(`use test`)
tk.MustExec(`set @@tidb_enable_non_prepared_plan_cache=1`)
tk.MustExec(`set @@tidb_opt_fix_control = "44830:ON"`)
tk.MustExec(`create table t1 (a int, b int, primary key(a, b))`) // multiple-column primary key
tk.MustExec(`insert into t1 values (1, 1), (2, 2), (3, 3)`)
tk.MustExec(`set @a=1, @b=2, @c=3`)

tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((1, 1), (2, 2), (3, 3))`).Sort().Check(testkit.Rows("1 1", "2 2", "3 3"))
tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((1, 1), (2, 2), (3, 3))`).Sort().Check(testkit.Rows("1 1", "2 2", "3 3"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1"))
tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((1, 1), (2, 2), (2, 2))`).Sort().Check(testkit.Rows("1 1", "2 2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0"))
tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((2, 2), (2, 2), (2, 2))`).Sort().Check(testkit.Rows("2 2"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0"))
tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((1, 1), (1, 1), (1, 1))`).Sort().Check(testkit.Rows("1 1"))
tk.MustQuery(`select * from t1 where 1=1 and (a, b) in ((1, 1), (1, 1), (1, 1))`).Sort().Check(testkit.Rows("1 1"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0"))
}

func TestPlanCacheSizeSwitch(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
88 changes: 88 additions & 0 deletions planner/core/plan_cache_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/pingcap/tidb/parser/ast"
"github.com/pingcap/tidb/parser/model"
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/planner/util"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/sessionctx/variable"
Expand Down Expand Up @@ -577,3 +578,90 @@ func checkTypesCompatibility4PC(tpsExpected, tpsActual []*types.FieldType) bool
}
return true
}

func isSafePointGetPath4PlanCache(sctx sessionctx.Context, path *util.AccessPath) bool {
// PointGet might contain some over-optimized assumptions, like `a>=1 and a<=1` --> `a=1`, but
// these assumptions may be broken after parameters change.

if isSafePointGetPath4PlanCacheScenario1(path) {
return true
}

// TODO: enable this fix control switch by default after more test cases are added.
if sctx != nil && sctx.GetSessionVars() != nil && sctx.GetSessionVars().OptimizerFixControl != nil {
v, ok := sctx.GetSessionVars().OptimizerFixControl[variable.TiDBOptFixControl44830]
if ok && variable.TiDBOptOn(v) && (isSafePointGetPath4PlanCacheScenario2(path) || isSafePointGetPath4PlanCacheScenario3(path)) {
return true
}
}

return false
}

func isSafePointGetPath4PlanCacheScenario1(path *util.AccessPath) bool {
// safe scenario 1: each column corresponds to a single EQ, `a=1 and b=2 and c=3` --> `[1, 2, 3]`
if len(path.Ranges) <= 0 || path.Ranges[0].Width() != len(path.AccessConds) {
return false
}
for _, accessCond := range path.AccessConds {
f, ok := accessCond.(*expression.ScalarFunction)
if !ok || f.FuncName.L != ast.EQ { // column = constant
return false
}
}
return true
}

func isSafePointGetPath4PlanCacheScenario2(path *util.AccessPath) bool {
// safe scenario 2: this Batch or PointGet is simply from a single IN predicate, `key in (...)`
if len(path.Ranges) <= 0 || len(path.AccessConds) != 1 {
return false
}
f, ok := path.AccessConds[0].(*expression.ScalarFunction)
if !ok || f.FuncName.L != ast.In {
return false
}
return len(path.Ranges) == len(f.GetArgs())-1 // no duplicated values in this in-list for safety.
}

func isSafePointGetPath4PlanCacheScenario3(path *util.AccessPath) bool {
// safe scenario 3: this Batch or PointGet is simply from a simple DNF like `key=? or key=? or key=?`
if len(path.Ranges) <= 0 || len(path.AccessConds) != 1 {
return false
}
f, ok := path.AccessConds[0].(*expression.ScalarFunction)
if !ok || f.FuncName.L != ast.LogicOr {
return false
}

dnfExprs := expression.FlattenDNFConditions(f)
if len(path.Ranges) != len(dnfExprs) {
// no duplicated values in this in-list for safety.
// e.g. `k=1 or k=2 or k=1` --> [[1, 1], [2, 2]]
return false
}

for _, expr := range dnfExprs {
f, ok := expr.(*expression.ScalarFunction)
if !ok {
return false
}
switch f.FuncName.L {
case ast.EQ: // (k=1 or k=2) --> [k=1, k=2]
case ast.LogicAnd: // ((k1=1 and k2=1) or (k1=2 and k2=2)) --> [k1=1 and k2=1, k2=2 and k2=2]
cnfExprs := expression.FlattenCNFConditions(f)
if path.Ranges[0].Width() != len(cnfExprs) { // not all key columns are specified
return false
}
for _, expr := range cnfExprs { // k1=1 and k2=1
f, ok := expr.(*expression.ScalarFunction)
if !ok || f.FuncName.L != ast.EQ {
return false
}
}
default:
return false
}
}
return true
}
3 changes: 3 additions & 0 deletions sessionctx/variable/session.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,9 @@ var (
TiDBOptFixControl44262 uint64 = 44262
// TiDBOptFixControl44389 controls whether to consider non-point ranges of some CNF item when building ranges.
TiDBOptFixControl44389 uint64 = 44389
// TiDBOptFixControl44830 controls whether to allow to cache Batch/PointGet from some complex scenarios.
// See #44830 for more details.
TiDBOptFixControl44830 uint64 = 44830
// TiDBOptFixControl44823 controls the maximum number of parameters for a query that can be cached in the Plan Cache.
TiDBOptFixControl44823 uint64 = 44823
// TiDBOptFixControl44855 controls whether to use a more accurate upper bound when estimating row count of index
Expand Down

0 comments on commit e091ee5

Please sign in to comment.