Skip to content

Commit

Permalink
planner: fix index join range display when executing the cached plan (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
xuyifangreeneyes committed Oct 10, 2022
1 parent 571b1f2 commit 1f5bddc
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 2 deletions.
3 changes: 3 additions & 0 deletions planner/core/exhaust_physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1598,7 +1598,10 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
func (ijHelper *indexJoinBuildHelper) updateBestChoice(ranges ranger.MutableRanges, path *util.AccessPath, accesses,
remained []expression.Expression, lastColManager *ColWithCmpFuncManager, usedColsLen int, rebuildMode bool) {
if rebuildMode { // rebuild the range for plan-cache, update the chosenRanges anyway
ijHelper.chosenPath = path
ijHelper.chosenRanges = ranges
ijHelper.chosenAccess = accesses
ijHelper.idxOff2KeyOff = ijHelper.curIdxOff2KeyOff
return
}

Expand Down
3 changes: 1 addition & 2 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7652,8 +7652,7 @@ func TestPlanCacheForIndexJoinRangeFallback(t *testing.T) {
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
rows = tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
// We don't limit range mem usage when rebuilding index join ranges for the cached plan. So [? aaaaaa,? aaaaaa], [? bbbbbb,? bbbbbb], [? cccccc,? cccccc] can be built.
// TODO: use the right range `range: decided by [eq(test.t1.a, test.t2.d) in(test.t1.b, aaaaaa, bbbbbb, cccccc)]` after https://github.com/pingcap/tidb/issues/38269 is fixed.
require.True(t, strings.Contains(rows[6][4].(string), "range: decided by [eq(test.t1.a, test.t2.d) in(test.t1.b, a, b, c)]"))
require.True(t, strings.Contains(rows[6][4].(string), "range: decided by [eq(test.t1.a, test.t2.d) in(test.t1.b, aaaaaa, bbbbbb, cccccc)]"))

// Test the plan with range fallback would not be put into cache.
tk.MustExec("prepare stmt2 from 'select /*+ inl_join(t1) */ * from t1 join t2 on t1.a = t2.d where t1.b in (?, ?, ?, ?, ?)'")
Expand Down
23 changes: 23 additions & 0 deletions planner/core/plan_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,23 @@ func RebuildPlan4CachedPlan(p Plan) error {
return rebuildRange(p)
}

func updateRange(p PhysicalPlan, ranges ranger.Ranges, rangeInfo string) {
switch x := p.(type) {
case *PhysicalTableScan:
x.Ranges = ranges
x.rangeInfo = rangeInfo
case *PhysicalIndexScan:
x.Ranges = ranges
x.rangeInfo = rangeInfo
case *PhysicalTableReader:
updateRange(x.TablePlans[0], ranges, rangeInfo)
case *PhysicalIndexReader:
updateRange(x.IndexPlans[0], ranges, rangeInfo)
case *PhysicalIndexLookUpReader:
updateRange(x.IndexPlans[0], ranges, rangeInfo)
}
}

// rebuildRange doesn't set mem limit for building ranges. There are two reasons why we don't restrict range mem usage here.
// 1. The cached plan must be able to build complete ranges under mem limit when it is generated. Hence we can just build
// ranges from x.AccessConditions. The only difference between the last ranges and new ranges is the change of parameter
Expand All @@ -327,6 +344,12 @@ func rebuildRange(p Plan) error {
if err := x.Ranges.Rebuild(); err != nil {
return err
}
if mutableRange, ok := x.Ranges.(*mutableIndexJoinRange); ok {
helper := mutableRange.buildHelper
rangeInfo := helper.buildRangeDecidedByInformation(helper.chosenPath.IdxCols, mutableRange.outerJoinKeys)
innerPlan := x.Children()[x.InnerChildIdx]
updateRange(innerPlan, x.Ranges.Range(), rangeInfo)
}
for _, child := range x.Children() {
err = rebuildRange(child)
if err != nil {
Expand Down
20 changes: 20 additions & 0 deletions planner/core/plan_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
plannercore "github.com/pingcap/tidb/planner/core"
"github.com/pingcap/tidb/testkit"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/util"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -110,3 +111,22 @@ func TestGeneralPlanCacheBasically(t *testing.T) {
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1")) // this plan is from plan-cache
}
}

func TestIssue38269(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec(`set @@tidb_enable_prepared_plan_cache=1`)
tk.MustExec("set @@tidb_enable_collect_execution_info=0")
tk.MustExec("use test")
tk.MustExec("create table t1(a int)")
tk.MustExec("create table t2(a int, b int, c int, index idx(a, b))")
tk.MustExec("prepare stmt1 from 'select /*+ inl_join(t2) */ * from t1 join t2 on t1.a = t2.a where t2.b in (?, ?, ?)'")
tk.MustExec("set @a = 10, @b = 20, @c = 30, @d = 40, @e = 50, @f = 60")
tk.MustExec("execute stmt1 using @a, @b, @c")
tk.MustExec("execute stmt1 using @d, @e, @f")
tkProcess := tk.Session().ShowProcess()
ps := []*util.ProcessInfo{tkProcess}
tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
rows := tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Rows()
require.Contains(t, rows[6][4], "range: decided by [eq(test.t2.a, test.t1.a) in(test.t2.b, 40, 50, 60)]")
}

0 comments on commit 1f5bddc

Please sign in to comment.