Skip to content

Commit

Permalink
planner: generate PointGet plans for prepare/execute statements (#42194…
Browse files Browse the repository at this point in the history 
…) (#42679)

close #42125
  • Loading branch information
@qw4990
qw4990 authored Mar 29, 2023
1 parent cc95a63 commit 6e854ff
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 16 deletions.
64 changes: 57 additions & 7 deletions executor/prepared_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,21 +272,17 @@ func TestIssue29850(t *testing.T) {
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&mockSessionManager1{PS: ps})
tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Check(testkit.Rows( // cannot use PointGet since it contains a range condition
`Selection_7 1.00 root ge(test.t.a, 1), le(test.t.a, 1)`,
`└─TableReader_6 1.00 root data:TableRangeScan_5`,
` └─TableRangeScan_5 1.00 cop[tikv] table:t range:[1,1], keep order:false, stats:pseudo`))
`Point_Get_5 1.00 root table:t handle:1`))
tk.MustQuery(`execute stmt using @a1, @a2`).Check(testkit.Rows("1", "2"))
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 * from t where a=? or a=?'`)
tk.MustQuery(`execute stmt using @a1, @a1`).Check(testkit.Rows("1"))
tkProcess = tk.Session().ShowProcess()
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&mockSessionManager1{PS: ps})
tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Check(testkit.Rows( // cannot use PointGet since it contains a or condition
`Selection_7 1.00 root or(eq(test.t.a, 1), eq(test.t.a, 1))`,
`└─TableReader_6 1.00 root data:TableRangeScan_5`,
` └─TableRangeScan_5 1.00 cop[tikv] table:t range:[1,1], keep order:false, stats:pseudo`))
`Point_Get_5 1.00 root table:t handle:1`))
tk.MustQuery(`execute stmt using @a1, @a2`).Check(testkit.Rows("1", "2"))
}

Expand Down Expand Up @@ -337,6 +333,60 @@ func TestIssue28064(t *testing.T) {
" └─TableRowIDScan_6(Probe) 0.00 cop[tikv] table:t28064 keep order:false, stats:pseudo"))
}

func TestIssue42125(t *testing.T) {
store, dom, err := newStoreWithBootstrap()
require.NoError(t, err)
orgEnable := plannercore.PreparedPlanCacheEnabled()
defer func() {
plannercore.SetPreparedPlanCache(orgEnable)
}()
plannercore.SetPreparedPlanCache(true)
tk := testkit.NewTestKit(t, store)
defer func() {
dom.Close()
require.NoError(t, store.Close())
}()

tk.MustExec("use test")
tk.MustExec("create table t (a int, b int, c int, unique key(a, b))")

// should use BatchPointGet
tk.MustExec("prepare st from 'select * from t where a=1 and b in (?, ?)'")
tk.MustExec("set @a=1, @b=2")
tk.MustExec("execute st using @a, @b")
tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&mockSessionManager1{PS: ps})
rows := tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
require.Equal(t, rows[0][0], "Batch_Point_Get_5") // use BatchPointGet
tk.MustExec("execute st using @a, @b")

// should use PointGet: unsafe PointGet
tk.MustExec("prepare st from 'select * from t where a=1 and b>=? and b<=?'")
tk.MustExec("set @a=1, @b=1")
tk.MustExec("execute st using @a, @b")
tkProcess = tk.Session().ShowProcess()
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&mockSessionManager1{PS: ps})
rows = tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
require.Equal(t, rows[0][0], "Point_Get_5") // use Point_Get_5
tk.MustExec("execute st using @a, @b")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0")) // cannot hit

// safe PointGet
tk.MustExec("prepare st from 'select * from t where a=1 and b=? and c<?'")
tk.MustExec("set @a=1, @b=1")
tk.MustExec("execute st using @a, @b")
tkProcess = tk.Session().ShowProcess()
ps = []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&mockSessionManager1{PS: ps})
rows = tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
require.Contains(t, rows[0][0], "Selection") // PointGet -> Selection
require.Contains(t, rows[1][0], "Point_Get")
tk.MustExec("execute st using @a, @b")
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1")) // can hit
}

func TestPreparePlanCache4Blacklist(t *testing.T) {
store, dom, err := newStoreWithBootstrap()
require.NoError(t, err)
Expand Down
16 changes: 9 additions & 7 deletions planner/core/find_best_task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,10 +798,6 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
}
canConvertPointGet := len(path.Ranges) > 0 && path.StoreType == kv.TiKV && ds.isPointGetConvertableSchema()

if canConvertPointGet && expression.MaybeOverOptimized4PlanCache(ds.ctx, path.AccessConds) {
canConvertPointGet = ds.canConvertToPointGetForPlanCache(path)
}

if canConvertPointGet && !path.IsIntHandlePath {
// We simply do not build [batch] point get for prefix indexes. This can be optimized.
canConvertPointGet = path.Index.Unique && !path.Index.HasPrefixIndex()
Expand Down Expand Up @@ -857,6 +853,13 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
} else {
pointGetTask = ds.convertToBatchPointGet(prop, candidate, hashPartColName)
}

// 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) {
ds.ctx.GetSessionVars().StmtCtx.SkipPlanCache = true
}

if !pointGetTask.invalid() {
cntPlan += 1
planCounter.Dec(1)
Expand Down Expand Up @@ -921,12 +924,11 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter
return
}

func (ds *DataSource) canConvertToPointGetForPlanCache(path *util.AccessPath) bool {
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.
// So for safety, we narrow down the scope and just generate PointGet in some particular and simple scenarios.

// scenario 1: each column corresponds to a single EQ, `a=1 and b=2 and c=3` --> `[1, 2, 3]`
// 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)
Expand Down
4 changes: 2 additions & 2 deletions planner/core/prepare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2078,7 +2078,7 @@ func (s *testPlanSerialSuite) TestPlanCachePointGetAndTableDual(c *C) {
tk.MustQuery("execute s2 using @a2, @a2").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
tk.MustQuery("execute s2 using @a2, @b2").Check(testkit.Rows("1 7777"))
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 t3(c1 int, c2 int, c3 int, unique key(c1), key(c2))")
tk.MustExec("insert into t3 values(2,1,1)")
Expand Down Expand Up @@ -2141,7 +2141,7 @@ func (s *testPrepareSuite) TestIssue26873(c *C) {
tk.MustQuery("execute stmt using @p").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
tk.MustQuery("execute stmt using @p").Check(testkit.Rows())
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("1"))
tk.MustQuery("select @@last_plan_from_cache").Check(testkit.Rows("0"))
}

func (s *testPrepareSuite) TestIssue29511(c *C) {
Expand Down

0 comments on commit 6e854ff

Please sign in to comment.