Skip to content

executor: do not build range for NullOuterVal in IndexLookUpJoin#8505

Merged
zz-jason merged 5 commits into
pingcap:masterfrom
XuHuaiyu:fix_inlj_range_from_handle
Nov 29, 2018
Merged

executor: do not build range for NullOuterVal in IndexLookUpJoin#8505
zz-jason merged 5 commits into
pingcap:masterfrom
XuHuaiyu:fix_inlj_range_from_handle

Conversation

@XuHuaiyu
Copy link
Copy Markdown
Contributor

@XuHuaiyu XuHuaiyu commented Nov 29, 2018

What problem does this PR solve?

fix #8499

Before this PR, the result of the following query is wrong.

CREATE TABLE `t1` (
  `a` int(11) NOT NULL,
  `b` int(11) DEFAULT NULL,
  PRIMARY KEY (`a`)
);
insert into t1 values(1, 0), (2, null);
CREATE TABLE `t2` (
  `a` int(11) NOT NULL,
  PRIMARY KEY (`a`)
);
insert into t2 values(0);
tidb> select /*+ TIDB_INLJ(t2)*/ * from t1 left join t2 on t1.b = t2.a;
+---+------+------+
| a | b    | a    |
+---+------+------+
| 1 |    0 |    0 |
| 1 |    0 |    0 |
| 2 | NULL | NULL |
+---+------+------+
3 rows in set (0.00 sec)

What is changed and how it works?

When the innerPlan is a TableReader, the range of PKHanle will be built in buildTableReaderForIndexJoin,
and NilDatum.GetInt64() will return 0. Thus we read the row with PkHandle=0 twice, and get a duplicate result line.

When the innerPlan is a IndexReader or IndexLookUpReader, we'll encode the NilDatum, thus no error happens.

Check List

Tests

  • Integration test

Code changes

  • Has exported function/method change

Side effects
none

Related changes

  • Need to cherry-pick to the release branch

This change is Reviewable

@XuHuaiyu XuHuaiyu added type/bugfix This PR fixes a bug. sig/execution SIG execution labels Nov 29, 2018
@XuHuaiyu XuHuaiyu changed the title executor: add check for Null-Handle when buildTableReaderForIndexJoin executor: add check for NilDatum when buildTableReaderForIndexJoin Nov 29, 2018
@XuHuaiyu
Copy link
Copy Markdown
Contributor Author

/run-all-tests

@XuHuaiyu
Copy link
Copy Markdown
Contributor Author

PTAL @zz-jason @winoros @eurekaka @lamxTyler

Comment thread executor/builder.go Outdated
Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 29, 2018
Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuHuaiyu we should cherry pick this change to release-2.1 and release-2.0

@zz-jason zz-jason added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Nov 29, 2018
eurekaka
eurekaka previously approved these changes Nov 29, 2018
Copy link
Copy Markdown
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eurekaka eurekaka added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 29, 2018
Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason
Copy link
Copy Markdown
Member

/run-all-tests

@XuHuaiyu XuHuaiyu changed the title executor: add check for NilDatum when buildTableReaderForIndexJoin executor: do not build range for NullOuterVal in IndexLookUpJoin Nov 29, 2018
@XuHuaiyu
Copy link
Copy Markdown
Contributor Author

/run-integration-ddl-test

@XuHuaiyu XuHuaiyu deleted the fix_inlj_range_from_handle branch December 4, 2018 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/release-blocker This issue blocks a release. Please solve it ASAP. sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

left join cause result duplicate

4 participants