Skip to content

Commit

Permalink
planner: fix uncorrect behavior of index join (#11724)
Browse files Browse the repository at this point in the history
  • Loading branch information
winoros authored and sre-bot committed Aug 16, 2019
1 parent 523b936 commit 79d43a5
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 78 deletions.
9 changes: 9 additions & 0 deletions executor/join_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1390,4 +1390,13 @@ func (s *testSuite2) TestIssue11544(c *C) {
tk.MustExec("insert into 11544t values(1)")
tk.MustExec("insert into 11544tt values(1, 'aaaaaaa'), (1, 'aaaabbb'), (1, 'aaaacccc')")
tk.MustQuery("select /*+ TIDB_INLJ(tt) */ * from 11544t t, 11544tt tt where t.a=tt.a and (tt.b = 'aaaaaaa' or tt.b = 'aaaabbb')").Check(testkit.Rows("1 1 aaaaaaa", "1 1 aaaabbb"))
tk.MustQuery("select /*+ TIDB_INLJ(tt) */ * from 11544t t, 11544tt tt where t.a=tt.a and tt.b in ('aaaaaaa', 'aaaabbb', 'aaaacccc')").Check(testkit.Rows("1 1 aaaaaaa", "1 1 aaaabbb", "1 1 aaaacccc"))
}

func (s *testSuite2) TestIssue11390(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("create table 11390t (k1 int unsigned, k2 int unsigned, key(k1, k2))")
tk.MustExec("insert into 11390t values(1, 1)")
tk.MustQuery("select /*+ TIDB_INLJ(t1, t2) */ * from 11390t t1, 11390t t2 where t1.k2 > 0 and t1.k2 = t2.k2 and t2.k1=1;").Check(testkit.Rows("1 1 1 1"))
}
118 changes: 45 additions & 73 deletions planner/core/exhaust_physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,34 +295,6 @@ func (p *LogicalJoin) getHashJoin(prop *property.PhysicalProperty, innerIdx int)
return hashJoin
}

// joinKeysMatchIndex checks whether the join key is in the index.
// It returns a slice a[] what a[i] means keys[i] is related with indexCols[a[i]], -1 for no matching column.
// It will return nil if there's no column that matches index.
func joinKeysMatchIndex(keys, indexCols []*expression.Column, colLengths []int) []int {
keyOff2IdxOff := make([]int, len(keys))
for i := range keyOff2IdxOff {
keyOff2IdxOff[i] = -1
}
// There should be at least one column in join keys which can match the index's column.
matched := false
tmpSchema := expression.NewSchema(keys...)
for i, idxCol := range indexCols {
if colLengths[i] != types.UnspecifiedLength {
continue
}
keyOff := tmpSchema.ColumnIndex(idxCol)
if keyOff == -1 {
continue
}
matched = true
keyOff2IdxOff[keyOff] = i
}
if !matched {
return nil
}
return keyOff2IdxOff
}

// When inner plan is TableReader, the parameter `ranges` will be nil. Because pk only have one column. So all of its range
// is generated during execution time.
func (p *LogicalJoin) constructIndexJoin(
Expand Down Expand Up @@ -451,7 +423,10 @@ func (p *LogicalJoin) getIndexJoinByOuterIdx(prop *property.PhysicalProperty, ou
continue
}
indexInfo := path.index
err := helper.analyzeLookUpFilters(indexInfo, ds, innerJoinKeys)
emptyRange, err := helper.analyzeLookUpFilters(indexInfo, ds, innerJoinKeys)
if emptyRange {
return nil
}
if err != nil {
logutil.BgLogger().Warn("build index join failed", zap.Error(err))
}
Expand Down Expand Up @@ -828,10 +803,10 @@ func (ijHelper *indexJoinBuildHelper) removeUselessEqAndInFunc(
return notKeyEqAndIn, nil
}

func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(indexInfo *model.IndexInfo, innerPlan *DataSource, innerJoinKeys []*expression.Column) error {
func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(indexInfo *model.IndexInfo, innerPlan *DataSource, innerJoinKeys []*expression.Column) (emptyRange bool, err error) {
idxCols, colLengths := expression.IndexInfo2Cols(innerPlan.schema.Columns, indexInfo)
if len(idxCols) == 0 {
return nil
return false, nil
}
accesses := make([]expression.Expression, 0, len(idxCols))
ijHelper.resetContextForIndex(innerJoinKeys, idxCols, colLengths)
Expand All @@ -841,31 +816,34 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(indexInfo *model.Inde
matchedKeyCnt := len(ijHelper.curPossibleUsedKeys)
// If no join key is matched while join keys actually are not empty. We don't choose index join for now.
if matchedKeyCnt <= 0 && len(innerJoinKeys) > 0 {
return nil
return false, nil
}
accesses = append(accesses, 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.
// A more strict check is applied later.
if lastColPos <= 0 {
return nil
return false, nil
}
// If all the index columns are covered by eq/in conditions, we don't need to consider other conditions anymore.
if lastColPos == len(idxCols) {
// If there's join key matched index column. Then choose hash join is always a better idea.
// e.g. select * from t1, t2 where t2.a=1 and t2.b=1. And t2 has index(a, b).
// If we don't have the following check, TiDB will build index join for this case.
if matchedKeyCnt <= 0 {
return nil
return false, nil
}
remained = append(remained, rangeFilterCandidates...)
ranges, err := ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nil, false)
ranges, emptyRange, err := ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nil, false)
if err != nil {
return err
return false, err
}
if emptyRange {
return true, nil
}
ijHelper.updateBestChoice(ranges, indexInfo, accesses, remained, nil)
return nil
return false, nil
}
lastPossibleCol := idxCols[lastColPos]
lastColManager := &ColWithCmpFuncManager{
Expand All @@ -880,37 +858,43 @@ func (ijHelper *indexJoinBuildHelper) analyzeLookUpFilters(indexInfo *model.Inde
// e.g. select * from t1, t2 where t2.a=1 and t2.b=1 and t2.c > 10 and t2.c < 20. And t2 has index(a, b, c).
// If we don't have the following check, TiDB will build index join for this case.
if matchedKeyCnt <= 0 {
return nil
return false, nil
}
colAccesses, colRemained := ranger.DetachCondsForColumn(ijHelper.join.ctx, rangeFilterCandidates, lastPossibleCol)
var ranges, nextColRange []*ranger.Range
var err error
if len(colAccesses) > 0 {
nextColRange, err = ranger.BuildColumnRange(colAccesses, ijHelper.join.ctx.GetSessionVars().StmtCtx, lastPossibleCol.RetType, colLengths[lastColPos])
if err != nil {
return err
return false, err
}
}
ranges, err = ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nextColRange, false)
ranges, emptyRange, err = ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nextColRange, false)
if err != nil {
return err
return false, err
}
if emptyRange {
return true, nil
}
remained = append(remained, colRemained...)
if colLengths[lastColPos] != types.UnspecifiedLength {
remained = append(remained, colAccesses...)
}
accesses = append(accesses, colAccesses...)
ijHelper.updateBestChoice(ranges, indexInfo, accesses, remained, nil)
return nil
return false, nil
}
accesses = append(accesses, lastColAccess...)
remained = append(remained, rangeFilterCandidates...)
ranges, err := ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nil, true)
ranges, emptyRange, err := ijHelper.buildTemplateRange(matchedKeyCnt, notKeyEqAndIn, nil, true)
if err != nil {
return err
return false, err
}
if emptyRange {
return true, nil
}
ijHelper.updateBestChoice(ranges, indexInfo, accesses, remained, lastColManager)
return nil
return false, nil
}

func (ijHelper *indexJoinBuildHelper) updateBestChoice(ranges []*ranger.Range, idxInfo *model.IndexInfo, accesses,
Expand All @@ -929,7 +913,7 @@ func (ijHelper *indexJoinBuildHelper) updateBestChoice(ranges []*ranger.Range, i
}
}

func (ijHelper *indexJoinBuildHelper) buildTemplateRange(matchedKeyCnt int, eqAndInFuncs []expression.Expression, nextColRange []*ranger.Range, haveExtraCol bool) (ranges []*ranger.Range, err error) {
func (ijHelper *indexJoinBuildHelper) buildTemplateRange(matchedKeyCnt int, eqAndInFuncs []expression.Expression, nextColRange []*ranger.Range, haveExtraCol bool) (ranges []*ranger.Range, emptyRange bool, err error) {
pointLength := matchedKeyCnt + len(eqAndInFuncs)
if nextColRange != nil {
for _, colRan := range nextColRange {
Expand All @@ -956,49 +940,37 @@ func (ijHelper *indexJoinBuildHelper) buildTemplateRange(matchedKeyCnt int, eqAn
HighVal: make([]types.Datum, pointLength, pointLength),
})
}
emptyRow := chunk.Row{}
sc := ijHelper.join.ctx.GetSessionVars().StmtCtx
for i, j := 0, 0; j < len(eqAndInFuncs); i++ {
// This position is occupied by join key.
if ijHelper.curIdxOff2KeyOff[i] != -1 {
continue
}
sf := eqAndInFuncs[j].(*expression.ScalarFunction)
// Deal with the first two args.
if _, ok := sf.GetArgs()[0].(*expression.Column); ok {
for _, ran := range ranges {
ran.LowVal[i], err = sf.GetArgs()[1].Eval(emptyRow)
if err != nil {
return nil, err
}
ran.HighVal[i] = ran.LowVal[i]
}
} else {
for _, ran := range ranges {
ran.LowVal[i], err = sf.GetArgs()[0].Eval(emptyRow)
if err != nil {
return nil, err
}
ran.HighVal[i] = ran.LowVal[i]
}
oneColumnRan, err := ranger.BuildColumnRange([]expression.Expression{eqAndInFuncs[j]}, sc, ijHelper.curNotUsedIndexCols[j].RetType, ijHelper.curNotUsedColLens[j])
if err != nil {
return nil, false, err
}
if len(oneColumnRan) == 0 {
return nil, true, nil
}
for _, ran := range ranges {
ran.LowVal[i] = oneColumnRan[0].LowVal[0]
ran.HighVal[i] = oneColumnRan[0].HighVal[0]
}
// If the length of in function's constant list is more than one, we will expand ranges.
curRangeLen := len(ranges)
for argIdx := 2; argIdx < len(sf.GetArgs()); argIdx++ {
for ranIdx := 1; ranIdx < len(oneColumnRan); ranIdx++ {
newRanges := make([]*ranger.Range, 0, curRangeLen)
for oldRangeIdx := 0; oldRangeIdx < curRangeLen; oldRangeIdx++ {
newRange := ranges[oldRangeIdx].Clone()
newRange.LowVal[i], err = sf.GetArgs()[argIdx].Eval(emptyRow)
if err != nil {
return nil, err
}
newRange.HighVal[i] = newRange.LowVal[i]
newRange.LowVal[i] = oneColumnRan[ranIdx].LowVal[0]
newRange.HighVal[i] = oneColumnRan[ranIdx].HighVal[0]
newRanges = append(newRanges, newRange)
}
ranges = append(ranges, newRanges...)
}
j++
}
return ranges, nil
return ranges, false, nil
}

// tryToGetIndexJoin will get index join by hints. If we can generate a valid index join by hint, the second return value
Expand Down
2 changes: 1 addition & 1 deletion planner/core/exhaust_physical_plans_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (s *testUnitTestSuit) TestIndexJoinAnalyzeLookUpFilters(c *C) {
c.Assert(err, IsNil)
joinNode.OtherConditions = others
helper := &indexJoinBuildHelper{join: joinNode, lastColManager: nil}
err = helper.analyzeLookUpFilters(idxInfo, dataSourceNode, tt.innerKeys)
_, err = helper.analyzeLookUpFilters(idxInfo, dataSourceNode, tt.innerKeys)
c.Assert(err, IsNil)
c.Assert(fmt.Sprintf("%v", helper.chosenRanges), Equals, tt.ranges, Commentf("test case: #%v", i))
c.Assert(fmt.Sprintf("%v", helper.idxOff2KeyOff), Equals, tt.idxOff2KeyOff)
Expand Down
28 changes: 26 additions & 2 deletions planner/core/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,28 @@ func MockUnsignedTable() *model.TableInfo {
{
Name: model.NewCIStr("b"),
Length: types.UnspecifiedLength,
Offset: 4,
Offset: 1,
},
},
State: model.StatePublic,
Unique: true,
},
{
Name: model.NewCIStr("b_c"),
Columns: []*model.IndexColumn{
{
Name: model.NewCIStr("b"),
Length: types.UnspecifiedLength,
Offset: 1,
},
{
Name: model.NewCIStr("c"),
Length: types.UnspecifiedLength,
Offset: 2,
},
},
State: model.StatePublic,
},
}
pkColumn := &model.ColumnInfo{
State: model.StatePublic,
Expand All @@ -296,11 +312,19 @@ func MockUnsignedTable() *model.TableInfo {
FieldType: newLongType(),
ID: 2,
}
col1 := &model.ColumnInfo{
State: model.StatePublic,
Offset: 2,
Name: model.NewCIStr("c"),
FieldType: newLongType(),
ID: 3,
}
pkColumn.Flag = mysql.PriKeyFlag | mysql.NotNullFlag | mysql.UnsignedFlag
// Column 'b', 'c', 'd', 'f', 'g' is not null.
col0.Flag = mysql.NotNullFlag
col1.Flag = mysql.UnsignedFlag
table := &model.TableInfo{
Columns: []*model.ColumnInfo{pkColumn, col0},
Columns: []*model.ColumnInfo{pkColumn, col0, col1},
Indices: indices,
Name: model.NewCIStr("t2"),
PKIsHandle: true,
Expand Down
10 changes: 8 additions & 2 deletions planner/core/physical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func (s *testPlanSuite) TestDAGPlanBuilderSimpleCase(c *C) {
},
{
sql: "select * from t2 use index(b) where b = 1 and a = 1",
best: "IndexReader(Index(t2.b)[[1,1]]->Sel([eq(test.t2.a, 1)]))",
best: "IndexLookUp(Index(t2.b)[[1,1]]->Sel([eq(test.t2.a, 1)]), Table(t2))",
},
}
for i, tt := range tests {
Expand Down Expand Up @@ -1559,13 +1559,19 @@ func (s *testPlanSuite) TestJoinHints(c *C) {
best: "LeftHashJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t1.b,test.t2.a)",
warning: "[planner:1815]Optimizer Hint /*+ INL_JOIN(t1) */ or /*+ TIDB_INLJ(t1) */ is inapplicable",
},
{
sql: "select /*+ TIDB_INLJ(t2) */ t1.b, t2.a from t2 t1, t2 t2 where t1.b=t2.b and t2.c=-1;",
best: "IndexJoin{IndexReader(Index(t2.b)[[NULL,+inf]])->TableReader(Table(t2)->Sel([eq(test.t2.c, -1)]))}(test.t2.b,test.t1.b)->Projection",
warning: "[planner:1815]Optimizer Hint /*+ INL_JOIN(t2) */ or /*+ TIDB_INLJ(t2) */ is inapplicable",
},
}
ctx := context.Background()
for i, test := range tests {
comment := Commentf("case:%v sql:%s", i, test)
stmt, err := s.ParseOneStmt(test.sql, "", "")
c.Assert(err, IsNil, comment)

se.GetSessionVars().StmtCtx.SetWarnings(nil)
p, err := planner.Optimize(ctx, se, stmt, s.is)
c.Assert(err, IsNil)
c.Assert(core.ToString(p), Equals, test.best)
Expand All @@ -1574,7 +1580,7 @@ func (s *testPlanSuite) TestJoinHints(c *C) {
if test.warning == "" {
c.Assert(len(warnings), Equals, 0)
} else {
c.Assert(len(warnings), Equals, 1)
c.Assert(len(warnings), Equals, 1, Commentf("%v", warnings))
c.Assert(warnings[0].Level, Equals, stmtctx.WarnLevelWarning)
c.Assert(warnings[0].Err.Error(), Equals, test.warning)
}
Expand Down
9 changes: 9 additions & 0 deletions util/ranger/ranger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,15 @@ func (s *testRangerSuite) TestColumnRange(c *C) {
resultStr: "[[\"a\",\"a\"]]",
length: 1,
},
// This test case cannot be simplified to [1, 3] otherwise the index join will executes wrongly.
{
colPos: 0,
exprStr: "a in (1, 2, 3)",
accessConds: "[in(test.t.a, 1, 2, 3)]",
filterConds: "",
resultStr: "[[1,1] [2,2] [3,3]]",
length: types.UnspecifiedLength,
},
}

ctx := context.Background()
Expand Down

0 comments on commit 79d43a5

Please sign in to comment.