Skip to content

Commit 72f52f3

Browse files
qw4990qw4990
authored
planner: update the plan cache strategy when expressions with parameters affect null-check (pingcap#40218)
close pingcap#38205, close pingcap#40093
1 parent 25dd54f commit 72f52f3

File tree

6 files changed

+103
-25
lines changed

6 files changed

+103
-25
lines changed

‎executor/explainfor_test.go

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -551,9 +551,9 @@ func TestIssue28259(t *testing.T) {
551551
ps = []*util.ProcessInfo{tkProcess}
552552
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
553553
res = tk.MustQuery("explain for connection " + strconv.FormatUint(tkProcess.ID, 10))
554-
require.Len(t, res.Rows(), 4)
555-
require.Regexp(t, ".*Selection.*", res.Rows()[0][0])
556-
require.Regexp(t, ".*IndexFullScan.*", res.Rows()[3][0])
554+
require.Len(t, res.Rows(), 3)
555+
require.Regexp(t, ".*Selection.*", res.Rows()[1][0])
556+
require.Regexp(t, ".*IndexFullScan.*", res.Rows()[2][0])
557557

558558
res = tk.MustQuery("explain format = 'brief' select col1 from UK_GCOL_VIRTUAL_18588 use index(UK_COL1) " +
559559
"where col1 between -1696020282760139948 and -2619168038882941276 or col1 < -4004648990067362699;")
@@ -589,11 +589,9 @@ func TestIssue28259(t *testing.T) {
589589
ps = []*util.ProcessInfo{tkProcess}
590590
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
591591
res = tk.MustQuery("explain for connection " + strconv.FormatUint(tkProcess.ID, 10))
592-
require.Len(t, res.Rows(), 5)
593-
require.Regexp(t, ".*Selection.*", res.Rows()[1][0])
594-
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])
595-
require.Regexp(t, ".*IndexReader.*", res.Rows()[2][0])
596-
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[4][0])
592+
require.Len(t, res.Rows(), 4)
593+
require.Regexp(t, ".*Selection.*", res.Rows()[2][0])
594+
require.Regexp(t, ".*IndexRangeScan.*", res.Rows()[3][0])
597595

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

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

865862
tk.MustExec("prepare stmt from 'select /*+ use_index_merge(t1) */ * from t1 where c=10 or (a >=? and a <= ?);';")
866863
tk.MustExec("set @a=9, @b=10, @c=11;")

‎expression/expression.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ func SplitDNFItems(onExpr Expression) []Expression {
816816
// If the Expression is a non-constant value, it means the result is unknown.
817817
func EvaluateExprWithNull(ctx sessionctx.Context, schema *Schema, expr Expression) Expression {
818818
if MaybeOverOptimized4PlanCache(ctx, []Expression{expr}) {
819-
return expr
819+
ctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.New("skip plan-cache: %v affects null check"))
820820
}
821821
if ctx.GetSessionVars().StmtCtx.InNullRejectCheck {
822822
expr, _ = evaluateExprWithNullInNullRejectCheck(ctx, schema, expr)

‎expression/expression_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ func TestEvaluateExprWithNullAndParameters(t *testing.T) {
7575
ltWithParam, err := newFunctionForTest(ctx, ast.LT, col0, param)
7676
require.NoError(t, err)
7777
res = EvaluateExprWithNull(ctx, schema, ltWithParam)
78-
_, isScalarFunc := res.(*ScalarFunction)
79-
require.True(t, isScalarFunc) // the expression with parameters is not evaluated
78+
_, isConst := res.(*Constant)
79+
require.True(t, isConst) // this expression is evaluated and skip-plan cache flag is set.
80+
require.True(t, ctx.GetSessionVars().StmtCtx.SkipPlanCache)
8081
}
8182

8283
func TestEvaluateExprWithNullNoChangeRetType(t *testing.T) {

‎planner/core/plan_cache_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,88 @@ func TestIssue38533(t *testing.T) {
221221
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
222222
}
223223

224+
func TestInvalidRange(t *testing.T) {
225+
store := testkit.CreateMockStore(t)
226+
tk := testkit.NewTestKit(t, store)
227+
tk.MustExec("use test")
228+
tk.MustExec("create table t (a int, key(a))")
229+
tk.MustExec("prepare st from 'select * from t where a>? and a<?'")
230+
tk.MustExec("set @l=100, @r=10")
231+
tk.MustExec("execute st using @l, @r")
232+
233+
tkProcess := tk.Session().ShowProcess()
234+
ps := []*util.ProcessInfo{tkProcess}
235+
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
236+
237+
tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
238+
[][]interface{}{{"TableDual_5"}}) // use TableDual directly instead of TableFullScan
239+
240+
tk.MustExec("execute st using @l, @r")
241+
tk.MustExec("execute st using @l, @r")
242+
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
243+
}
244+
245+
func TestIssue40093(t *testing.T) {
246+
store := testkit.CreateMockStore(t)
247+
tk := testkit.NewTestKit(t, store)
248+
tk.MustExec("use test")
249+
tk.MustExec("create table t1 (a int, b int)")
250+
tk.MustExec("create table t2 (a int, b int, key(b, a))")
251+
tk.MustExec("prepare st from 'select * from t1 left join t2 on t1.a=t2.a where t2.b in (?)'")
252+
tk.MustExec("set @b=1")
253+
tk.MustExec("execute st using @b")
254+
255+
tkProcess := tk.Session().ShowProcess()
256+
ps := []*util.ProcessInfo{tkProcess}
257+
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
258+
259+
tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
260+
[][]interface{}{
261+
{"Projection_9"},
262+
{"└─HashJoin_21"},
263+
{" ├─IndexReader_26(Build)"},
264+
{" │ └─IndexRangeScan_25"}, // RangeScan instead of FullScan
265+
{" └─TableReader_24(Probe)"},
266+
{" └─Selection_23"},
267+
{" └─TableFullScan_22"},
268+
})
269+
270+
tk.MustExec("execute st using @b")
271+
tk.MustExec("execute st using @b")
272+
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
273+
}
274+
275+
func TestIssue38205(t *testing.T) {
276+
store := testkit.CreateMockStore(t)
277+
tk := testkit.NewTestKit(t, store)
278+
tk.MustExec("use test")
279+
tk.MustExec("CREATE TABLE `item` (`id` int, `vid` varbinary(16), `sid` int)")
280+
tk.MustExec("CREATE TABLE `lv` (`item_id` int, `sid` int, KEY (`sid`,`item_id`))")
281+
282+
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 (?, ?)'")
283+
tk.MustExec("set @a=1, @b='1', @c='3'")
284+
tk.MustExec("execute stmt using @a, @b, @c")
285+
286+
tkProcess := tk.Session().ShowProcess()
287+
ps := []*util.ProcessInfo{tkProcess}
288+
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
289+
290+
tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).CheckAt([]int{0},
291+
[][]interface{}{
292+
{"IndexJoin_10"},
293+
{"├─TableReader_19(Build)"},
294+
{"│ └─Selection_18"},
295+
{"│ └─TableFullScan_17"}, // RangeScan instead of FullScan
296+
{"└─IndexReader_9(Probe)"},
297+
{" └─Selection_8"},
298+
{" └─IndexRangeScan_7"},
299+
})
300+
301+
tk.MustExec("execute stmt using @a, @b, @c")
302+
tk.MustExec("execute stmt using @a, @b, @c")
303+
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
304+
}
305+
224306
func TestIgnoreInsertStmt(t *testing.T) {
225307
store := testkit.CreateMockStore(t)
226308
tk := testkit.NewTestKit(t, store)

‎planner/core/prepare_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,7 @@ func TestPlanCachePointGetAndTableDual(t *testing.T) {
19391939
tk.MustQuery("execute s0 using @a0, @b0, @a0").Check(testkit.Rows())
19401940
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
19411941
tk.MustQuery("execute s0 using @a0, @a0, @b0").Check(testkit.Rows("0000 7777 1"))
1942-
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))
1942+
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
19431943

19441944
tk.MustExec("create table t1(c1 varchar(20), c2 varchar(20), c3 bigint(20), primary key(c1, c2))")
19451945
tk.MustExec("insert into t1 values('0000','7777',1)")

‎util/ranger/detacher.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -581,9 +581,8 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex
581581
points[offset] = rb.intersection(points[offset], rb.build(cond, collator), collator)
582582
if len(points[offset]) == 0 { // Early termination if false expression found
583583
if expression.MaybeOverOptimized4PlanCache(sctx, conditions) {
584-
// cannot return an empty-range for plan-cache since the range may become non-empty as parameters change
585-
// for safety, return the whole conditions in this case
586-
return nil, conditions, nil, nil, false
584+
// `a>@x and a<@y` --> `invalid-range if @x>=@y`
585+
sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: some parameters may be overwritten"))
587586
}
588587
return nil, nil, nil, nil, true
589588
}
@@ -606,9 +605,8 @@ func ExtractEqAndInCondition(sctx sessionctx.Context, conditions []expression.Ex
606605
accesses[i] = nil
607606
} else if len(points[i]) == 0 { // Early termination if false expression found
608607
if expression.MaybeOverOptimized4PlanCache(sctx, conditions) {
609-
// cannot return an empty-range for plan-cache since the range may become non-empty as parameters change
610-
// for safety, return the whole conditions in this case
611-
return nil, conditions, nil, nil, false
608+
// `a>@x and a<@y` --> `invalid-range if @x>=@y`
609+
sctx.GetSessionVars().StmtCtx.SetSkipPlanCache(errors.Errorf("skip plan-cache: some parameters may be overwritten"))
612610
}
613611
return nil, nil, nil, nil, true
614612
} else {

0 commit comments

Comments
 (0)