executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. #12889

wshwsh12 · 2019-10-23T02:38:42Z

What problem does this PR solve?

In buildProjectionForIndexJoin function, it calls ProjectionExec.Open() and recursive calls IndexLookUpExecutor.Open(). In this function, IndexLookUp will re-calculate kvRanges using e.ranges.
In this case, e.ranges is not initiated and will build a empty range.

What is changed and how it works?

Only init ProjectExec and don't recursive init childExec.
The childExec will be initiated in other buildxxxForIndexJoin function.

Check List

Tests

Unit test
Integration test
Manual test

Change the cost model to choose IndexMergeJoin.
Run the following SQL in master and this pr.

use test;
drop table if exists t1, t2;
create table t1(a int, b date, c float, primary key(a, b));
create table t2(a int primary key);
insert into t1 values(1, '2017-11-29', 2.2);
insert into t2 values(1);
select /*+ TIDB_INLJ(t1) */ * from t1 join t2 on t1.a=t2.a;

2.1 In master,

tidb> select /*+ TIDB_INLJ(t1) */ * from t1 join t2 on t1.a=t2.a;
Empty set (0.00 sec)

tidb> desc analyze select /*+ TIDB_INLJ(t1) */ * from t1 join t2 on t1.a=t2.a;
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+------------------------------------------------------+-----------+
| id                       | count | task      | operator info                                                                                   | execution info                                       | memory    |
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+------------------------------------------------------+-----------+
| IndexMergeJoin_25        | 1.00  | root      | inner join, inner:Projection_23, outer key:Column#5, inner key:Column#1                         | time:546.102µs, loops:2, rows:0                      | 745 Bytes |
| ├─Projection_23          | 1.00  | root      | Column#1, Column#2, Column#3                                                                    | time:73.202µs, loops:1, rows:0                       | N/A       |
| │ └─IndexLookUp_22       | 1.00  | root      |                                                                                                 | time:72.229µs, loops:1, rows:0                       | 0 Bytes   |
| │   ├─IndexScan_20       | 1.00  | cop[tikv] | table:t1, index:a, b, range: decided by [eq(Column#1, Column#5)], keep order:true, stats:pseudo | time:0s, loops:0, rows:0                             | N/A       |
| │   └─TableScan_21       | 1.00  | cop[tikv] | table:t1, keep order:false, stats:pseudo                                                        | time:0s, loops:0, rows:0                             | N/A       |
| └─TableReader_29         | 1.00  | root      | data:TableScan_28                                                                               | time:108.032µs, loops:2, rows:1, rpc time:136.84µs   | 127 Bytes |
|   └─TableScan_28         | 1.00  | cop[tikv] | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo                                     | time:73.076µs, loops:2, rows:1                       | N/A       |
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+------------------------------------------------------+-----------+
7 rows in set (0.00 sec)

2.2 In this pr,

tidb> select /*+ TIDB_INLJ(t1) */ * from t1 join t2 on t1.a=t2.a;
+---+------------+------+---+
| a | b          | c    | a |
+---+------------+------+---+
| 1 | 2017-11-29 |  2.2 | 1 |
+---+------------+------+---+
1 row in set (0.01 sec)

tidb> desc analyze select /*+ TIDB_INLJ(t1) */ * from t1 join t2 on t1.a=t2.a;
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+-------------------------------------------------------+---------------+
| id                       | count | task      | operator info                                                                                   | execution info                                        | memory        |
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+-------------------------------------------------------+---------------+
| IndexMergeJoin_25        | 1.00  | root      | inner join, inner:Projection_23, outer key:Column#5, inner key:Column#1                         | time:2.865241ms, loops:4, rows:2                      | 745 Bytes     |
| ├─Projection_23          | 1.00  | root      | Column#1, Column#2, Column#3                                                                    | time:502.299µs, loops:2, rows:1                       | N/A           |
| │ └─IndexLookUp_22       | 1.00  | root      |                                                                                                 | time:497.121µs, loops:2, rows:1, rpc time:123.668µs   | 9.73046875 KB |
| │   ├─IndexScan_20       | 1.00  | cop[tikv] | table:t1, index:a, b, range: decided by [eq(Column#1, Column#5)], keep order:true, stats:pseudo | time:49.991µs, loops:2, rows:1                        | N/A           |
| │   └─TableScan_21       | 1.00  | cop[tikv] | table:t1, keep order:false, stats:pseudo                                                        | time:35.144µs, loops:2, rows:1                        | N/A           |
| └─TableReader_29         | 1.00  | root      | data:TableScan_28                                                                               | time:454.372µs, loops:2, rows:1, rpc time:1.558265ms  | 127 Bytes     |
|   └─TableScan_28         | 1.00  | cop[tikv] | table:t2, range:[-inf,+inf], keep order:false, stats:pseudo                                     | time:216.994µs, loops:2, rows:1                       | N/A           |
+--------------------------+-------+-----------+-------------------------------------------------------------------------------------------------+-------------------------------------------------------+---------------+
7 rows in set (0.00 sec)

Code changes

Side effects

Increased code complexity

Related changes

Release note

lzmhhh123

LGTM

codecov · 2019-10-23T02:53:38Z

Codecov Report

Merging #12889 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #12889   +/-   ##
=======================================
  Coverage   79.98%   79.98%           
=======================================
  Files         465      465           
  Lines      107188   107188           
=======================================
  Hits        85729    85729           
  Misses      14994    14994           
  Partials     6465     6465

wshwsh12 · 2019-10-23T02:53:39Z

/run-all-tests

XuHuaiyu

Please check whether this bug fix should cherry-pick to release-3.1 and release-3.0

XuHuaiyu · 2019-10-23T03:05:31Z

executor/projection.go

+}
+
+func (e *ProjectionExec) open(ctx context.Context) error {
+	// We have to initialize execution resources in here instead of in function "Open",


This comment is unnecessary.

XuHuaiyu · 2019-10-23T03:07:11Z

executor/builder.go

@@ -2230,7 +2230,7 @@ func (builder *dataReaderBuilder) buildProjectionForIndexJoin(ctx context.Contex
 	if int64(v.StatsCount()) < int64(builder.ctx.GetSessionVars().MaxChunkSize) {
 		e.numWorkers = 0
 	}
-	err = e.Open(ctx)
+	err = e.open(ctx)


Can we try to refine the test case in the issue, and ensure IndexMergeJoin can be chosen?

wshwsh12 · 2019-10-24T03:10:51Z

/rebuild

XuHuaiyu

LGTM

sre-bot · 2019-10-24T03:23:28Z

/run-all-tests

sre-bot · 2019-10-24T03:32:33Z

/run-all-tests

…ect/tidb into feature-add-udf-support * 'feature-add-udf-support' of https://github.com/JustProject/tidb: (26 commits) *: fix bug that the kill command doesn't work when the killed session is waiting for the pessimistic lock (pingcap#12852) executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. (pingcap#12889) planner, executor: support create view on union (pingcap#12595) planner/cascades: introduce TransformationID in cascades planner (pingcap#12879) executor: fix data race in test (pingcap#12910) executor: reuse chunk row for insert on duplicate update (pingcap#12847) ddl: speed up tests (pingcap#12888) executor: speed up test (pingcap#12896) expression: implement vectorized evaluation for `builtinSecondSig` (pingcap#12886) expression: implement vectorized evaluation for `builtinJSONObjectSig` (pingcap#12663) expression: speed up builtinRepeatSig by using MergeNulls (pingcap#12674) expression: speed up unit tests under the expression package (pingcap#12887) store,kv: snapshot doesn't cache the non-exists kv entries lead to poor 'insert ignore' performance (pingcap#12872) executor: fix data race in `GetDirtyTable()` (pingcap#12767) domain: increase TTL to reduce the occurrence of reporting min startTS errors (pingcap#12578) executor: split test for speed up (pingcap#12881) executor: fix inconsistent of grants privileges with MySQL when executing `grant all on ...` (pingcap#12330) expression: implement vectorized evaluation for `builtinJSONUnquoteSig` (pingcap#12841) tune grpc connection count between tidb and tikv (pingcap#12884) Makefile: change test parallel to 8 (pingcap#12885) ...

…can't get result. (pingcap#12889)

wshwsh12 requested review from XuHuaiyu and lzmhhh123 October 23, 2019 02:38

lzmhhh123 reviewed Oct 23, 2019

View reviewed changes

lzmhhh123 added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 23, 2019

wshwsh12 added type/bugfix This PR fixes a bug. sig/execution SIG execution labels Oct 23, 2019

XuHuaiyu reviewed Oct 23, 2019

View reviewed changes

wshwsh12 added 3 commits October 24, 2019 10:09

fix bug

5a4012a

typo

f42e134

remove comments

a45097f

wshwsh12 force-pushed the imj_proj_wrong branch from b378621 to a45097f Compare October 24, 2019 02:10

XuHuaiyu approved these changes Oct 24, 2019

View reviewed changes

wshwsh12 added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 24, 2019

Merge branch 'master' into imj_proj_wrong

033c0cb

sre-bot merged commit bb1c5c8 into pingcap:master Oct 24, 2019

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

executor: fix the projection upon the indexLookUp in indexLookUpJoin …

d7414ed

…can't get result. (pingcap#12889)

wshwsh12 deleted the imj_proj_wrong branch April 21, 2020 05:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. #12889

executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. #12889

wshwsh12 commented Oct 23, 2019 •

edited

Loading

lzmhhh123 left a comment

codecov bot commented Oct 23, 2019 •

edited

Loading

wshwsh12 commented Oct 23, 2019

XuHuaiyu left a comment

XuHuaiyu Oct 23, 2019

XuHuaiyu Oct 23, 2019

wshwsh12 commented Oct 24, 2019

XuHuaiyu left a comment

sre-bot commented Oct 24, 2019

sre-bot commented Oct 24, 2019

executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. #12889

executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. #12889

Conversation

wshwsh12 commented Oct 23, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

lzmhhh123 left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 23, 2019 • edited Loading

Codecov Report

wshwsh12 commented Oct 23, 2019

XuHuaiyu left a comment

Choose a reason for hiding this comment

XuHuaiyu Oct 23, 2019

Choose a reason for hiding this comment

XuHuaiyu Oct 23, 2019

Choose a reason for hiding this comment

wshwsh12 commented Oct 24, 2019

XuHuaiyu left a comment

Choose a reason for hiding this comment

sre-bot commented Oct 24, 2019

sre-bot commented Oct 24, 2019

wshwsh12 commented Oct 23, 2019 •

edited

Loading

codecov bot commented Oct 23, 2019 •

edited

Loading