Skip to content

Commit

Permalink
planner: update the plan cache strategy when expressions with paramet…
Browse files Browse the repository at this point in the history
…ers affect null-check (#40218) (#40689)

close #38205, close #40093
  • Loading branch information
ti-chi-bot authored Feb 14, 2023
1 parent 0aa864d commit 4969a15
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 25 deletions.
27 changes: 12 additions & 15 deletions executor/explainfor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,9 +551,9 @@ func TestIssue28259(t *testing.T) {
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
res = tk.MustQuery("explain for connection " + strconv.FormatUint(tkProcess.ID, 10))
require.Len(t, res.Rows(), 4)
require.Regexp(t, ".*Selection.*", res.Rows()[0][0])
require.Regexp(t, ".*IndexFullScan.*", res.Rows()[3][0])
require.Len(t, res.Rows(), 3)
require.Regexp(t, ".*Selection.*", res.Rows()[1][0])
require.Regexp(t, ".*IndexFullScan.*", res.Rows()[2][0])

res = tk.MustQuery("explain format = 'brief' select col1 from UK_GCOL_VIRTUAL_18588 use index(UK_COL1) " +
"where col1 between -1696020282760139948 and -2619168038882941276 or col1 < -4004648990067362699;")
Expand Down Expand Up @@ -589,11 +589,9 @@ func TestIssue28259(t *testing.T) {
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
res = tk.MustQuery("explain for connection " + strconv.FormatUint(tkProcess.ID, 10))
require.Len(t, res.Rows(), 5)
require.Regexp(t, ".*Selection.*", res.Rows()[1][0])
require.Equal(t, "lt(test.t.b, 1), or(and(ge(test.t.a, 2), le(test.t.a, 1)), lt(test.t.a, 1))", res.Rows()[1][4])
require.Regexp(t, ".*IndexReader.*", res.Rows()[2][0])
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[4][0])
require.Len(t, res.Rows(), 4)
require.Regexp(t, ".*Selection.*", res.Rows()[2][0])
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[3][0])

res = tk.MustQuery("explain format = 'brief' select a from t use index(idx) " +
"where (a between 0 and 2 or a < 2) and b < 1;")
Expand Down Expand Up @@ -636,12 +634,11 @@ func TestIssue28259(t *testing.T) {
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
res = tk.MustQuery("explain for connection " + strconv.FormatUint(tkProcess.ID, 10))
require.Len(t, res.Rows(), 6)
require.Regexp(t, ".*Selection.*", res.Rows()[1][0])
require.Regexp(t, ".*IndexLookUp.*", res.Rows()[2][0])
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[3][0])
require.Regexp(t, ".*Selection.*", res.Rows()[4][0])
require.Regexp(t, ".*TableRowIDScan.*", res.Rows()[5][0])
require.Len(t, res.Rows(), 5)
require.Regexp(t, ".*IndexLookUp.*", res.Rows()[1][0])
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[2][0])
require.Regexp(t, ".*Selection.*", res.Rows()[3][0])
require.Regexp(t, ".*TableRowIDScan.*", res.Rows()[4][0])

res = tk.MustQuery("explain format = 'brief' select /*+ USE_INDEX(t, idx) */ a from t use index(idx) " +
"where (a between 0 and 2 or a < 2) and b < 1;")
Expand Down Expand Up @@ -860,7 +857,7 @@ func TestIndexMerge4PlanCache(t *testing.T) {
tk.MustExec("prepare stmt from 'select /*+ use_index_merge(t1) */ * from t1 where c=? or (b=? and (a >= ? and a <= ?));';")
tk.MustQuery("execute stmt using @a, @a, @b, @a").Check(testkit.Rows("10 10 10"))
tk.MustQuery("execute stmt using @b, @b, @b, @b").Check(testkit.Rows("11 11 11"))
tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("1"))
tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("0"))

tk.MustExec("prepare stmt from 'select /*+ use_index_merge(t1) */ * from t1 where c=10 or (a >=? and a <= ?);';")
tk.MustExec("set @a=9, @b=10, @c=11;")
Expand Down
3 changes: 2 additions & 1 deletion expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,8 @@ func SplitDNFItems(onExpr Expression) []Expression {
// If the Expression is a non-constant value, it means the result is unknown.
func EvaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expression) Expression {
if MaybeOverOptimized4PlanCache(ctx, []Expression{expr}) {
return expr
ctx.GetSessionVars().StmtCtx.SkipPlanCache = true
ctx.GetSessionVars().StmtCtx.AppendWarning(errors.New("skip plan-cache: %v affects null check"))
}
if ctx.GetSessionVars().StmtCtx.InNullRejectCheck {
expr, _ = evaluateExprWithNullInNullRejectCheck(ctx, schema, expr)
Expand Down
5 changes: 3 additions & 2 deletions expression/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ func TestEvaluateExprWithNullAndParameters(t *testing.T) {
ltWithParam, err := newFunctionForTest(ctx, ast.LT, col0, param)
require.NoError(t, err)
res = EvaluateExprWithNull(ctx, schema, ltWithParam)
_, isScalarFunc := res.(*ScalarFunction)
require.True(t, isScalarFunc) // the expression with parameters is not evaluated
_, isConst := res.(*Constant)
require.True(t, isConst) // this expression is evaluated and skip-plan cache flag is set.
require.True(t, ctx.GetSessionVars().StmtCtx.SkipPlanCache)
}

func TestEvaluateExprWithNullNoChangeRetType(t *testing.T) {
Expand Down
82 changes: 82 additions & 0 deletions planner/core/plan_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,88 @@ func TestIssue38533(t *testing.T) {
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

func TestInvalidRange(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t (a int, key(a))")
tk.MustExec("prepare st from 'select * from t where a>? and a<?'")
tk.MustExec("set @l=100, @r=10")
tk.MustExec("execute st using @l, @r")

tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})

tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
[][]interface{}{{"TableDual_5"}}) // use TableDual directly instead of TableFullScan

tk.MustExec("execute st using @l, @r")
tk.MustExec("execute st using @l, @r")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

func TestIssue40093(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t1 (a int, b int)")
tk.MustExec("create table t2 (a int, b int, key(b, a))")
tk.MustExec("prepare st from 'select * from t1 left join t2 on t1.a=t2.a where t2.b in (?)'")
tk.MustExec("set @b=1")
tk.MustExec("execute st using @b")

tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})

tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
[][]interface{}{
{"Projection_9"},
{"└─HashJoin_21"},
{" ├─IndexReader_26(Build)"},
{" │ └─IndexRangeScan_25"}, // RangeScan instead of FullScan
{" └─TableReader_24(Probe)"},
{" └─Selection_23"},
{" └─TableFullScan_22"},
})

tk.MustExec("execute st using @b")
tk.MustExec("execute st using @b")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

func TestIssue38205(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("CREATE TABLE `item` (`id` int, `vid` varbinary(16), `sid` int)")
tk.MustExec("CREATE TABLE `lv` (`item_id` int, `sid` int, KEY (`sid`,`item_id`))")

tk.MustExec("prepare stmt from 'SELECT /*+ TIDB_INLJ(lv, item) */ * FROM lv LEFT JOIN item ON lv.sid = item.sid AND lv.item_id = item.id WHERE item.sid = ? AND item.vid IN (?, ?)'")
tk.MustExec("set @a=1, @b='1', @c='3'")
tk.MustExec("execute stmt using @a, @b, @c")

tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})

tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
[][]interface{}{
{"IndexJoin_10"},
{"├─TableReader_19(Build)"},
{"│ └─Selection_18"},
{"│ └─TableFullScan_17"}, // RangeScan instead of FullScan
{"└─IndexReader_9(Probe)"},
{" └─Selection_8"},
{" └─IndexRangeScan_7"},
})

tk.MustExec("execute stmt using @a, @b, @c")
tk.MustExec("execute stmt using @a, @b, @c")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

func TestIgnoreInsertStmt(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
2 changes: 1 addition & 1 deletion planner/core/prepare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1940,7 +1940,7 @@ func TestPlanCachePointGetAndTableDual(t *testing.T) {
tk.MustQuery("execute s0 using @a0, @b0, @a0").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
tk.MustQuery("execute s0 using @a0, @a0, @b0").Check(testkit.Rows("0000 7777 1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("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)")
Expand Down
12 changes: 6 additions & 6 deletions util/ranger/detacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,9 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex
points[offset] = rb.intersection(points[offset], rb.build(cond, collator), collator)
if len(points[offset]) == 0 { // Early termination if false expression found
if expression.MaybeOverOptimized4PlanCache(sctx, conditions) {
// cannot return an empty-range for plan-cache since the range may become non-empty as parameters change
// for safety, return the whole conditions in this case
return nil, conditions, nil, nil, false
// `a>@x and a<@y` --> `invalid-range if @x>=@y`
sctx.GetSessionVars().StmtCtx.SkipPlanCache = true
sctx.GetSessionVars().StmtCtx.AppendWarning(errors.Errorf("skip plan-cache: some parameters may be overwritten"))
}
return nil, nil, nil, nil, true
}
Expand All @@ -606,9 +606,9 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex
accesses[i] = nil
} else if len(points[i]) == 0 { // Early termination if false expression found
if expression.MaybeOverOptimized4PlanCache(sctx, conditions) {
// cannot return an empty-range for plan-cache since the range may become non-empty as parameters change
// for safety, return the whole conditions in this case
return nil, conditions, nil, nil, false
// `a>@x and a<@y` --> `invalid-range if @x>=@y`
sctx.GetSessionVars().StmtCtx.SkipPlanCache = true
sctx.GetSessionVars().StmtCtx.AppendWarning(errors.Errorf("skip plan-cache: some parameters may be overwritten"))
}
return nil, nil, nil, nil, true
} else {
Expand Down

0 comments on commit 4969a15

Please sign in to comment.