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: plan cache supports Batch/PointGet converted from (primary keys) in ((...), ...) (#44838) #49734

Merged
Show file tree
Hide file tree
Changes from all commits
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
22 changes: 1 addition & 21 deletions planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,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 @@ -1123,26 +1123,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
4 changes: 2 additions & 2 deletions planner/core/plan_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func rebuildRange(p Plan) error {
if err != nil {
return err
}
if len(ranges.Ranges) == 0 || len(ranges.AccessConds) != len(x.AccessConditions) {
if len(ranges.Ranges) != 1 || len(ranges.AccessConds) != len(x.AccessConditions) {
return errors.New("failed to rebuild range: the length of the range has changed")
}
for i := range x.IndexValues {
Expand All @@ -447,7 +447,7 @@ func rebuildRange(p Plan) error {
if err != nil {
return err
}
if len(ranges) == 0 {
if len(ranges) != 1 {
return errors.New("failed to rebuild range: the length of the range has changed")
}
x.Handle = kv.IntHandle(ranges[0].LowVal[0].GetInt64())
Expand Down
44 changes: 44 additions & 0 deletions planner/core/plan_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,3 +529,47 @@ func TestIssue42150(t *testing.T) {
tk.MustExec("execute st")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))
}

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
}
89 changes: 89 additions & 0 deletions planner/core/plan_cache_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import (
"unsafe"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/parser"
"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/variable"
"github.com/pingcap/tidb/types"
Expand Down Expand Up @@ -454,3 +456,90 @@ func GetPreparedStmt(stmt *ast.ExecuteStmt, vars *variable.SessionVars) (*PlanCa
}
return nil, ErrStmtNotFound
}

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
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,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
)

// GetOptimizerFixControlValue returns the specified value of the optimizer fix control.
Expand Down