executor: support index_look_up_merge_join to speed up index_look_up_join #9571

lzmhhh123 · 2019-03-06T02:49:35Z

What problem does this PR solve?

Support index_look_up_merge_join for index_look_up_join, reference issue #8470.

What is changed and how it works?

In the process of index_look_up_join execution, if the inner table is PhysicalIndexScan and the join keys of the inner table are the suffix of the index, then we can choose index_look_up_merge_join.

Some details:

Because the merge tasks returned from inner workers are unsorted, the results of index_look_up_merge_join can't keep the order of the outer table by join keys.
The time complexity of index_look_up_merge_join are same as index_look_up_join. But it eliminates the process of the hash map. So in theory, it will speed up.

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change
Has exported variable/fields change

Side effects

Increased code complexity

Related changes

Need to cherry-pick to the release branch
Need to be included in the release note

codecov-io · 2019-03-07T05:46:19Z

Codecov Report

Merging #9571 into master will decrease coverage by 0.4687%.
The diff coverage is 77.5641%.

@@               Coverage Diff                @@
##             master      #9571        +/-   ##
================================================
- Coverage   81.4123%   80.9435%   -0.4688%     
================================================
  Files           423        419         -4     
  Lines         90700      89723       -977     
================================================
- Hits          73841      72625      -1216     
- Misses        11568      11828       +260     
+ Partials       5291       5270        -21

…hhh123/tidb into dev/index_look_up_merge_join

lzmhhh123 · 2019-03-11T07:06:27Z

PTAL. @zz-jason @XuHuaiyu

lzmhhh123 · 2019-03-12T08:52:14Z

I test the index_look_up_merge_join with TPC-H data.

The sql is:

select /*+ tidb_inlj(orders, lineitem) */
	lineitem.L_ORDERKEY,
	lineitem.L_LINENUMBER,
	orders.O_ORDERKEY
	from
		orders
		left join lineitem
	on orders.O_ORDERKEY = lineitem.L_ORDERKEY
	order by orders.O_ORDERKEY;

The memory cost is about 20% lower than index_look_up_join by eliminating the hash map.
And execution time is similar to index_look_up_join, improve a little.

Here's Frame gragh:

executor/index_lookup_merge_join.go

XuHuaiyu · 2019-03-21T06:21:24Z

executor/builder.go

@@ -1529,33 +1529,108 @@ func (b *executorBuilder) buildIndexLookUpJoin(v *plannercore.PhysicalIndexJoin)
 	if defaultValues == nil {
 		defaultValues = make([]types.Datum, len(innerTypes))
 	}
+	outerKeyCols := make([]int, len(v.OuterJoinKeys))


Would outerKeyColIdx be clearer?

XuHuaiyu · 2019-03-21T07:30:43Z

executor/builder.go

+	// If the inner join keys are the suffix set of the index, then return the IndexLookUpMergeJoin
+	var isIndexScan bool
+	var is *plannercore.PhysicalIndexScan
+	if ir, ok := innerPlan.(*plannercore.PhysicalIndexReader); ok {


What about innerPlan.(*plannercore.PhysicalTableReader) && IndexPlans[0].(*PhysicalTableScan) is true?

If code like this? How to get the IndexScan?

Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>

lzmhhh123 · 2019-07-03T04:06:28Z

ready to be reviewed. @XuHuaiyu @zz-jason @lamxTyler @eurekaka @qw4990

executor/index_lookup_merge_join.go

executor/builder.go

XuHuaiyu · 2019-07-10T05:38:25Z

executor/index_lookup_merge_join.go

+		sort.Slice(rows, func(i, j int) bool {
+			for id, joinKey := range omw.joinKeys {
+				cmp, _, err := omw.compareFuncs[id](omw.ctx, joinKey, joinKey, rows[i], rows[j])
+				terror.Log(err)


Will ignoring this error make the result wrong?

It's as same as index_lookup_join. Look at

tidb/executor/index_lookup_join.go

Line 563 in 2ea24e0

// We only compare rows with the same type, no error to return.

executor/index_lookup_merge_join.go

zz-jason · 2019-03-18T13:05:51Z

executor/index_lookup_merge_join.go

+	"github.com/pingcap/tidb/util/memory"
+	"github.com/pingcap/tidb/util/ranger"
+	"go.uber.org/zap"
+	"golang.org/x/net/context"


can be removed.

zz-jason · 2019-07-17T07:45:33Z

planner/core/exhaust_physical_plans.go

+	if ir, ok := innerPlan.(*PhysicalIndexReader); ok {
+		is, isIndexScan = ir.IndexPlans[0].(*PhysicalIndexScan)
+	}
+	if ilr, ok := innerPlan.(*PhysicalIndexLookUpReader); ok {


should we also consider PhysicalTableReader?

zz-jason · 2019-07-17T07:55:43Z

planner/core/exhaust_physical_plans.go

+			// or the outer join keys are NOT the prefix of the prop items,
+			// then we can't execute merge join.
+			if canKeepOuterOrder || needOuterSort {
+				is.KeepOrder = true


should we also set IndexLookupReader or IndexReader to keep order?

zz-jason · 2019-07-17T08:13:51Z

planner/core/exhaust_physical_plans.go

+			}
+			// canKeepOuterOrder means whether the prop items are the prefix of the outer join keys.
+			canKeepOuterOrder := len(propItems) <= len(join.OuterJoinKeys)
+			for i := range propItems {


for i := 0; i < len(propItems) && canKeepOuterOrder; i++ { }

zz-jason · 2019-07-17T09:02:25Z

planner/core/exhaust_physical_plans.go

+			// then we can't execute merge join.
+			if canKeepOuterOrder || needOuterSort {
+				is.KeepOrder = true
+				join.IsMergeJoin = true


How about providing a new physical operator named IndexMergeJoin?

XuHuaiyu · 2019-07-11T07:34:42Z

planner/core/exhaust_physical_plans.go

+	if ilr, ok := innerPlan.(*PhysicalIndexLookUpReader); ok {
+		is, isIndexScan = ilr.IndexPlans[0].(*PhysicalIndexScan)
+	}
+	if isIndexScan {


if !isIndexScan {
return
}

XuHuaiyu · 2019-07-11T07:34:51Z

planner/core/exhaust_physical_plans.go

+				break
+			}
+		}
+		if isInnerKeysPrefix {


XuHuaiyu · 2019-07-17T07:39:51Z

executor/index_lookup_merge_join.go

+	if err != nil {
+		return err
+	}
+	if err != nil {


XuHuaiyu · 2019-07-17T08:22:31Z

executor/index_lookup_merge_join.go

+	return 0, nil
+}
+
+func (imw *innerMergeWorker) constructDatumLookupKeys(task *lookUpMergeJoinTask) ([]*indexJoinLookUpContent, error) {


Can we reuse the existing innerWorker.constructDatumLookupKey since the logic of these two functions are the same?

XuHuaiyu · 2019-07-17T08:30:55Z

executor/index_lookup_merge_join.go

+					ranger.CutDatumByPrefixLen(&dLookUpKey.keys[i], imw.colLens[i], imw.rowTypes[imw.keyCols[i]])
+				}
+			}
+			// dLookUpKey is sorted and deduplicated at sortAndDedupLookUpContents.


useless comment

XuHuaiyu · 2019-07-17T08:59:58Z

executor/index_lookup_merge_join.go

+		outerRow := task.outerResult.GetRow(task.cursor)
+		if task.sameKeyIter == nil || task.sameKeyIter.Current() == task.sameKeyIter.End() {
+			cmpResult := -1
+			if len(task.outerMatch) > 0 && !task.outerMatch[task.cursor] {


Will len(task.outerMatch) == 0?

XuHuaiyu · 2019-07-17T09:10:52Z

executor/index_lookup_merge_join.go

+		return nil
+	}
+
+	task.innerIter = chunk.NewIterator4Chunk(task.innerResult.GetChunk(task.innerCursor))


Can we use task.innerIter = chunk.NewIterator4List ?

XuHuaiyu · 2019-07-17T09:11:16Z

executor/index_lookup_merge_join.go

+	}
+
+	task.innerIter = chunk.NewIterator4Chunk(task.innerResult.GetChunk(task.innerCursor))
+	task.innerIter.Begin()


innerRow := task.innerIter.Begin()

XuHuaiyu · 2019-07-17T09:33:21Z

executor/index_lookup_merge_join.go

+		if task.sameKeyIter == nil || task.sameKeyIter.Current() == task.sameKeyIter.End() {
+			cmpResult := -1
+			if len(task.outerMatch) > 0 && !task.outerMatch[task.cursor] {
+				task.cursor++


Can we use outerIter instead of cursor?

XuHuaiyu · 2019-07-17T09:35:58Z

executor/index_lookup_merge_join.go

+	task.sameKeyRows = task.sameKeyRows[:0]
+	task.sameKeyIter = chunk.NewIterator4Slice(task.sameKeyRows)
+	task.sameKeyIter.Begin()
+	if task.innerCursor >= task.innerResult.NumChunks() {


move this to the first line

lzmhhh123 added 6 commits November 21, 2018 18:27

ci

d209485

initialization

abb1dd0

Merge branch 'master' into dev/index_look_up_merge_join

8f20e84

finish core

280ebc7

add comments

ac24f82

debug

ec91f35

lzmhhh123 added type/enhancement sig/execution SIG execution labels Mar 6, 2019

lzmhhh123 changed the title ~~Executor: Support index_look_up_merge_join to speed up index_look_up_join~~ executor: Support index_look_up_merge_join to speed up index_look_up_join Mar 6, 2019

lzmhhh123 added 2 commits March 7, 2019 13:27

improve

0dadc5e

debug

9db06f6

lzmhhh123 and others added 10 commits March 7, 2019 20:22

debug

551ff53

debug

b9d6415

debug

b00365c

debug

da586ee

debug

925df37

Merge branch 'master' into dev/index_look_up_merge_join

70a6790

change merge worker into inner worker

9d9015e

Merge branch 'dev/index_look_up_merge_join' of https://github.com/lzm…

12f1d31

…hhh123/tidb into dev/index_look_up_merge_join

make results to chan

531a028

debug

06320c8

lzmhhh123 added 4 commits March 11, 2019 20:37

improve

8063340

reduce chan size

f53e06e

debug

1781313

improve

f381771

lzmhhh123 added 2 commits March 12, 2019 18:55

improve

75d75c0

improve

e2f0936

alivxxx reviewed Mar 19, 2019

View reviewed changes

executor/index_lookup_merge_join.go Outdated Show resolved Hide resolved

executor/index_lookup_merge_join.go Outdated Show resolved Hide resolved

executor/index_lookup_merge_join.go Outdated Show resolved Hide resolved

lzmhhh123 added 5 commits March 19, 2019 19:31

remove unnecessary judgement

bea33cd

remove context import from golang/x

a5acaf5

change

5fdd79c

split joiner

f4e9fc8

debug

b4b222f

XuHuaiyu reviewed Mar 21, 2019

View reviewed changes

Merge branch 'master' into dev/index_look_up_merge_join

b74396d

lzmhhh123 requested review from XuHuaiyu and eurekaka July 2, 2019 05:00

lzmhhh123 added 3 commits July 2, 2019 16:46

plug-in index join enhance code

6c3888b

adapt bug fix pingcap#10244

1aaf860

fix bug

b29e4ec

lzmhhh123 force-pushed the dev/index_look_up_merge_join branch from f44d7d5 to b29e4ec Compare July 2, 2019 11:45

add outerjoin required size control

56d45f8

Signed-off-by: lzmhhh123 <lzmhhh123@gmail.com>

XuHuaiyu reviewed Jul 10, 2019

View reviewed changes

lzmhhh123 added 3 commits July 11, 2019 14:35

address comments

eb55d55

fix ci

a4b4d08

fix explain test

4ad2c89

lzmhhh123 force-pushed the dev/index_look_up_merge_join branch from 2752248 to 4ad2c89 Compare July 15, 2019 06:44

lzmhhh123 added 2 commits July 15, 2019 14:52

Merge branch 'master' into dev/index_look_up_merge_join

97c88e3

fix index pre_length bug in indexjoin

4a2156b

zz-jason reviewed Jul 17, 2019

View reviewed changes

XuHuaiyu reviewed Jul 17, 2019

View reviewed changes

lzmhhh123 mentioned this pull request Jul 19, 2019

planner: support index_lookup_merge_join in physical plan. #11338

Merged

eurekaka removed their request for review July 23, 2019 06:27

qw4990 removed their request for review July 25, 2019 08:54

lzmhhh123 closed this Aug 1, 2019

lzmhhh123 mentioned this pull request Sep 4, 2019

executor: support index lookup merge join in executor. #12024

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: support index_look_up_merge_join to speed up index_look_up_join #9571

executor: support index_look_up_merge_join to speed up index_look_up_join #9571

lzmhhh123 commented Mar 6, 2019

codecov-io commented Mar 7, 2019 •

edited by codecov bot

lzmhhh123 commented Mar 11, 2019 •

edited

lzmhhh123 commented Mar 12, 2019

XuHuaiyu Mar 21, 2019

XuHuaiyu Mar 21, 2019

lzmhhh123 Mar 21, 2019

lzmhhh123 commented Jul 3, 2019

XuHuaiyu Jul 10, 2019

lzmhhh123 Jul 11, 2019

zz-jason Mar 18, 2019

zz-jason Jul 17, 2019

zz-jason Jul 17, 2019

zz-jason Jul 17, 2019

zz-jason Jul 17, 2019

XuHuaiyu Jul 11, 2019

XuHuaiyu Jul 11, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

XuHuaiyu Jul 17, 2019

executor: support index_look_up_merge_join to speed up index_look_up_join #9571

executor: support index_look_up_merge_join to speed up index_look_up_join #9571

Conversation

lzmhhh123 commented Mar 6, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

codecov-io commented Mar 7, 2019 • edited by codecov bot

Codecov Report

lzmhhh123 commented Mar 11, 2019 • edited

lzmhhh123 commented Mar 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzmhhh123 commented Jul 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 7, 2019 •

edited by codecov bot

lzmhhh123 commented Mar 11, 2019 •

edited