Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plan: use Column.UniqueID in conditionChecker of ranger #8236

Merged
merged 4 commits into from
Nov 12, 2018

Conversation

eurekaka
Copy link
Contributor

@eurekaka eurekaka commented Nov 8, 2018

What problem does this PR solve?

Fix #8223

What is changed and how it works?

  • Compare UniqueID of Column in ranger instead of ColName, then even if the column name is incorrect, correct index scan can be chosen;
  • Keep correct column name in projection elimination.

Both approaches can fix the bug independently, but seems we'd better to have them all.

We have to keep the original column name restore logic in resolveColumnAndReplace in order to maintain the compatibility with MySQL for this query:

mysql> select distinct a, (select b from tb where b = ta.a) from ta;
+---+-----------------------------------+
| a | (select b from tb where b = ta.a) |
+---+-----------------------------------+
| 1 |                                 1 |
+---+-----------------------------------+
1 row in set (0.00 sec)

if we remove line 54 and 56 from TiDB source code:

 51 func resolveColumnAndReplace(origin *expression.Column, replace map[string]*expression.Column) {
 52     dst := replace[string(origin.HashCode(nil))]
 53     if dst != nil {
 54         colName, retType := origin.ColName, origin.RetType
 55         *origin = *dst
 56         origin.ColName, origin.RetType = colName, retType
 57     }
 58 }

we would get result like:

mysql> select distinct a, (select b from tb where b = ta.a) from ta;
+---+---+
| a | b |
+---+---+
| 1 | 1 |
+---+---+
1 row in set (0.00 sec)

Check List

Tests

  • Unit test

Side effects

N/A

Related changes

  • Need to cherry-pick to the release branch

@eurekaka eurekaka added status/WIP type/bugfix This PR fixes a bug. sig/planner SIG: Planner labels Nov 8, 2018
@eurekaka
Copy link
Contributor Author

eurekaka commented Nov 9, 2018

/run-all-tests

@eurekaka
Copy link
Contributor Author

eurekaka commented Nov 9, 2018

/run-all-tests

Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu XuHuaiyu added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 12, 2018
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@XuHuaiyu XuHuaiyu merged commit f7d8ca6 into pingcap:master Nov 12, 2018
@eurekaka eurekaka deleted the ifnull_tbl_scan branch November 12, 2018 05:21
zz-jason pushed a commit to zz-jason/tidb that referenced this pull request Mar 7, 2019
XuHuaiyu pushed a commit to XuHuaiyu/tidb that referenced this pull request Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table scan is chosen while index scan is available when IfNull is eliminated
3 participants