Skip to content

Commit

Permalink
planner: fix the issue IndexJoin returns wrong results when using pla…
Browse files Browse the repository at this point in the history
…n-cache in some cases (pingcap#29349)
  • Loading branch information
qw4990 authored Nov 3, 2021
1 parent ee2adfd commit 599785d
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
15 changes: 9 additions & 6 deletions planner/core/exhaust_physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1366,13 +1366,13 @@ func (mr *mutableIndexJoinRange) Rebuild() error {
return nil
}

func (ijHelper *indexJoinBuildHelper) createMutableIndexJoinRange(notKeyEqAndIn []expression.Expression, ranges []*ranger.Range, path *util.AccessPath, innerKeys, outerKeys []*expression.Column) ranger.MutableRanges {
func (ijHelper *indexJoinBuildHelper) createMutableIndexJoinRange(relatedExprs []expression.Expression, ranges []*ranger.Range, path *util.AccessPath, innerKeys, outerKeys []*expression.Column) ranger.MutableRanges {
// if the plan-cache is enabled and these ranges depend on some parameters, we have to rebuild these ranges after changing parameters
if expression.MaybeOverOptimized4PlanCache(ijHelper.join.ctx, notKeyEqAndIn) {
if expression.MaybeOverOptimized4PlanCache(ijHelper.join.ctx, relatedExprs) {
// assume that path, innerKeys and outerKeys will not be modified in the follow-up process
return &mutableIndexJoinRange{
ranges: ranges,
buildHelper: ijHelper,
buildHelper: &indexJoinBuildHelper{innerPlan: ijHelper.innerPlan, join: ijHelper.join},
path: path,
innerJoinKeys: innerKeys,
outerJoinKeys: outerKeys,
Expand All @@ -1386,6 +1386,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
return false, nil
}
accesses := make([]expression.Expression, 0, len(path.IdxCols))
relatedExprs := make([]expression.Expression, 0, len(path.IdxCols)) // all expressions related to the chosen range
ijHelper.resetContextForIndex(innerJoinKeys, path.IdxCols, path.IdxColLens, outerJoinKeys)
notKeyEqAndIn, remained, rangeFilterCandidates := ijHelper.findUsefulEqAndInFilters(innerPlan)
var remainedEqAndIn []expression.Expression
Expand All @@ -1396,6 +1397,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
return false, nil
}
accesses = append(accesses, notKeyEqAndIn...)
relatedExprs = append(relatedExprs, notKeyEqAndIn...)
remained = append(remained, remainedEqAndIn...)
lastColPos := matchedKeyCnt + len(notKeyEqAndIn)
// There should be some equal conditions. But we don't need that there must be some join key in accesses here.
Expand All @@ -1419,7 +1421,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
if emptyRange {
return true, nil
}
mutableRange := ijHelper.createMutableIndexJoinRange(notKeyEqAndIn, ranges, path, innerJoinKeys, outerJoinKeys)
mutableRange := ijHelper.createMutableIndexJoinRange(relatedExprs, ranges, path, innerJoinKeys, outerJoinKeys)
ijHelper.updateBestChoice(mutableRange, path, accesses, remained, nil, lastColPos, rebuildMode)
return false, nil
}
Expand All @@ -1446,6 +1448,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
if err != nil {
return false, err
}
relatedExprs = append(relatedExprs, colAccesses...)
}
ranges, emptyRange, err = ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nextColRange, false)
if err != nil {
Expand All @@ -1462,7 +1465,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
if len(colAccesses) > 0 {
lastColPos = lastColPos + 1
}
mutableRange := ijHelper.createMutableIndexJoinRange(notKeyEqAndIn, ranges, path, innerJoinKeys, outerJoinKeys)
mutableRange := ijHelper.createMutableIndexJoinRange(relatedExprs, ranges, path, innerJoinKeys, outerJoinKeys)
ijHelper.updateBestChoice(mutableRange, path, accesses, remained, nil, lastColPos, rebuildMode)
return false, nil
}
Expand All @@ -1475,7 +1478,7 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(path *util.AccessPath
if emptyRange {
return true, nil
}
mutableRange := ijHelper.createMutableIndexJoinRange(notKeyEqAndIn, ranges, path, innerJoinKeys, outerJoinKeys)
mutableRange := ijHelper.createMutableIndexJoinRange(relatedExprs, ranges, path, innerJoinKeys, outerJoinKeys)
ijHelper.updateBestChoice(mutableRange, path, accesses, remained, lastColManager, lastColPos+1, rebuildMode)
return false, nil
}
Expand Down
34 changes: 34 additions & 0 deletions planner/core/prepare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,40 @@ func (s *testPlanSerialSuite) TestPlanCacheHitInfo(c *C) {
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("0"))
}

func (s *testPlanSerialSuite) TestIssue29303(c *C) {
defer testleak.AfterTest(c)()
store, dom, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
tk := testkit.NewTestKit(c, store)
orgEnable := core.PreparedPlanCacheEnabled()
defer func() {
dom.Close()
err = store.Close()
c.Assert(err, IsNil)
core.SetPreparedPlanCache(orgEnable)
}()
core.SetPreparedPlanCache(true)

tk.Se, err = session.CreateSession4TestWithOpt(store, &session.Opt{
PreparedPlanCache: kvcache.NewSimpleLRUCache(100, 0.1, math.MaxUint64),
})

tk.MustExec(`set tidb_enable_clustered_index=on`)
tk.MustExec(`use test`)
tk.MustExec(`drop table if exists PK_MULTI_COL_360`)
tk.MustExec(`CREATE TABLE PK_MULTI_COL_360 (
COL1 blob NOT NULL,
COL2 char(1) NOT NULL,
PRIMARY KEY (COL1(5),COL2) /*T![clustered_index] CLUSTERED */)`)
tk.MustExec(`INSERT INTO PK_MULTI_COL_360 VALUES ('�', '龂')`)
tk.MustExec(`prepare stmt from 'SELECT/*+ INL_JOIN(t1, t2) */ * FROM PK_MULTI_COL_360 t1 JOIN PK_MULTI_COL_360 t2 ON t1.col1 = t2.col1 WHERE t2.col2 BETWEEN ? AND ? AND t1.col2 BETWEEN ? AND ?'`)
tk.MustExec(`set @a="捲", @b="颽", @c="睭", @d="詼"`)
tk.MustQuery(`execute stmt using @a,@b,@c,@d`).Check(testkit.Rows())
tk.MustExec(`set @a="龂", @b="龂", @c="龂", @d="龂"`)
tk.MustQuery(`execute stmt using @a,@b,@c,@d`).Check(testkit.Rows("� 龂 � 龂"))
tk.MustQuery(`select @@last_plan_from_cache`).Check(testkit.Rows("1"))
}

func (s *testPlanSerialSuite) TestIssue28942(c *C) {
defer testleak.AfterTest(c)()
store, dom, err := newStoreWithBootstrap()
Expand Down

0 comments on commit 599785d

Please sign in to comment.