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.

Sign up for GitHub

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

Jump to bottom

planner: add index optimizer hint #11746

Merged
merged 12 commits into from
Aug 23, 2019
Merged

planner: add index optimizer hint #11746

merged 12 commits into from
Aug 23, 2019

Conversation

foreyes
Copy link
Contributor

@foreyes foreyes commented Aug 15, 2019 •
edited
Loading

What problem does this PR solve?

Support SQL hint use index in comment-style optimizer hint.

What is changed and how it works?

Add new hint field into planbuilder, and add matched ast.IndexHint into ast.TableName when build data source.

mysql> explain select /*+ INDEX(t, c_d_e) */ a from t t1;
+-------------------+----------+------+----------------------------------------------------------------------------+
| id                | count    | task | operator info                                                              |
+-------------------+----------+------+----------------------------------------------------------------------------+
| IndexReader_5     | 10000.00 | root | index:IndexScan_4                                                          |
| └─IndexScan_4     | 10000.00 | cop  | table:t1, index:c, d, e, range:[NULL,+inf], keep order:false, stats:pseudo |
+-------------------+----------+------+----------------------------------------------------------------------------+
2 rows in set (0.00 sec)

Check List

Tests

  • Unit test

@foreyes
Copy link
Contributor Author

foreyes commented Aug 15, 2019

/run-all-tests

@codecov
Copy link

codecov bot commented Aug 15, 2019 •
edited
Loading

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11746   +/-   ##
===========================================
  Coverage   81.9727%   81.9727%           
===========================================
  Files           435        435           
  Lines         96027      96027           
===========================================
  Hits          78716      78716           
  Misses        11847      11847           
  Partials       5464       5464

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #11746 into master will decrease coverage by 0.0021%.
The diff coverage is 82.8571%.

@@               Coverage Diff                @@
##             master     #11746        +/-   ##
================================================
- Coverage   81.4277%   81.4256%   -0.0022%     
================================================
  Files           433        433                
  Lines         93193      93220        +27     
================================================
+ Hits          75885      75905        +20     
- Misses        11877      11880         +3     
- Partials       5431       5435         +4

@foreyes
Copy link
Contributor Author

foreyes commented Aug 16, 2019

PTAL. @lamxTyler @lzmhhh123

@foreyes foreyes changed the title [WIP]planner: add index optimizer hint planner: add index optimizer hint Aug 16, 2019
lzmhhh123
lzmhhh123 reviewed Aug 16, 2019
View reviewed changes
planner/core/logical_plan_builder.go Outdated Show resolved Hide resolved
planner/core/logical_plan_builder.go Outdated Show resolved Hide resolved
planner/core/planbuilder.go Outdated Show resolved Hide resolved
eurekaka
eurekaka reviewed Aug 16, 2019
View reviewed changes
planner/core/logical_plan_builder.go Outdated Show resolved Hide resolved
planner/core/planbuilder.go Show resolved Hide resolved
@foreyes
Copy link
Contributor Author

foreyes commented Aug 19, 2019

/run-all-tests

1 similar comment
@foreyes
Copy link
Contributor Author

foreyes commented Aug 19, 2019

/run-all-tests

@foreyes foreyes force-pushed the dev/add_index_hint branch 2 times, most recently from 930d783 to 1a85b8e Compare August 19, 2019 05:49
@foreyes foreyes mentioned this pull request Aug 19, 2019
Merged
eurekaka
eurekaka reviewed Aug 20, 2019
View reviewed changes
Copy link
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 the status/LGT1 Indicates that a PR has LGTM 1. label Aug 20, 2019
@foreyes foreyes requested a review from lzmhhh123 August 23, 2019 07:36
@foreyes
Copy link
Contributor Author

foreyes commented Aug 23, 2019

/run-all-tests

@foreyes
Copy link
Contributor Author

foreyes commented Aug 23, 2019

/run-unit-test

lzmhhh123
lzmhhh123 approved these changes Aug 23, 2019
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM.

@lzmhhh123 lzmhhh123 added status/all tests passed 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 Aug 23, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Aug 23, 2019

/run-all-tests

@sre-bot sre-bot merged commit cc04cd9 into pingcap:master Aug 23, 2019
@foreyes foreyes deleted the dev/add_index_hint branch August 25, 2019 07:59
foreyes added a commit to foreyes/tidb that referenced this pull request Sep 16, 2019
@foreyes
planner: add index optimizer hint (pingcap#11746)
6833d8c
@foreyes foreyes mentioned this pull request Sep 16, 2019
Merged
@zz-jason zz-jason mentioned this pull request Oct 10, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alivxxx alivxxx alivxxx left review comments

@eurekaka eurekaka eurekaka left review comments

@lzmhhh123 lzmhhh123 lzmhhh123 approved these changes

Assignees
No one assigned
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@foreyes @sre-bot @eurekaka @alivxxx @lzmhhh123