planner: choose TableScan when use an empty index hint #12037

francis0407 · 2019-09-05T05:08:23Z

What problem does this PR solve?

fix #12033.

schema:

mysql> create table t(a int, b int primary key, index idx_a(a));

before this pr:

mysql> explain select a from t use index();
+-------------------+----------+------+---------------------------------------------------------------------+
| id                | count    | task | operator info                                                       |
+-------------------+----------+------+---------------------------------------------------------------------+
| IndexReader_7     | 10000.00 | root | index:IndexScan_6                                                   |
| └─IndexScan_6     | 10000.00 | cop  | table:t, index:a, range:[NULL,+inf], keep order:false, stats:pseudo |
+-------------------+----------+------+---------------------------------------------------------------------+

after this pr:

mysql> explain select a from t use index();
+-------------------+----------+------+------------------------------------------------------------+
| id                | count    | task | operator info                                              |
+-------------------+----------+------+------------------------------------------------------------+
| TableReader_5     | 10000.00 | root | data:TableScan_4                                           |
| └─TableScan_4     | 10000.00 | cop  | table:t, range:[-inf,+inf], keep order:false, stats:pseudo |
+-------------------+----------+------+------------------------------------------------------------+

What is changed and how it works?

Check List

Tests

Unit test

codecov · 2019-09-05T05:13:46Z

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12037   +/-   ##
===========================================
  Coverage   81.8853%   81.8853%           
===========================================
  Files           447        447           
  Lines         97159      97159           
===========================================
  Hits          79559      79559           
  Misses        12071      12071           
  Partials       5529       5529

winoros · 2019-09-05T05:29:31Z

planner/core/planbuilder.go

+			// For example: `select a from t use index()`.
+			if path := getTablePath(publicPaths); path != nil {
+				if hint.HintType == ast.HintIgnore {
+					ignored = append(ignored, path)


This ignore case doesn't make sense?

Sure. I'll remove this.

I think this is another issue. I tried ignore index() in MySQL, it returns a syntax Error.

mysql> explain select b from tt ignore index(); ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1

But TiDB allows such syntax.

alivxxx · 2019-09-05T05:36:18Z

planner/core/planbuilder.go

@@ -558,6 +558,19 @@ func (b *PlanBuilder) getPossibleAccessPaths(indexHints []*ast.IndexHint, tblInf
 		}

 		hasScanHint = true
+		if hint.IndexNames == nil {


Could you also consider the comment style hint like index(t)?

winoros · 2019-09-05T06:08:47Z

planner/core/planbuilder.go

+			// For example: `select a from t use index()`.
+			if path := getTablePath(publicPaths); path != nil {
+				hasUseOrForce = true
+				path.forced = true


You need check whether is ignore or force.
Or you'll set table path as forced although no index is ignored by IGNORE INDEX()

francis0407 · 2019-09-05T06:20:52Z

/rebuild

francis0407 · 2019-09-05T06:38:15Z

/run-all-tests

francis0407 · 2019-09-05T06:43:35Z

/rebuild

francis0407 · 2019-09-05T07:04:23Z

/run-all-tests

francis0407 · 2019-09-05T07:48:19Z

/run-all-tests

francis0407 · 2019-09-05T08:06:53Z

/rebuild

zz-jason · 2019-09-05T12:49:17Z

planner/core/planbuilder.go

@@ -558,6 +558,15 @@ func (b *PlanBuilder) getPossibleAccessPaths(indexHints []*ast.IndexHint, tblInf
 		}

 		hasScanHint = true
+		if hint.IndexNames == nil && hint.HintType != ast.HintIgnore {


How about adding a comment? From MySQL document:

It is syntactically valid to omit index_list for USE INDEX, which means “use no indexes.” Omitting index_list for FORCE INDEX or IGNORE INDEX is a syntax error.

…use_index

alivxxx

LGTM

lzmhhh123

LGTM.

sre-bot · 2019-09-06T08:38:22Z

/run-all-tests

francis0407 added 2 commits September 5, 2019 12:59

choose TableScan when use an empty index hint

05d95ff

remove unused blank line

aa2d13a

francis0407 added type/bugfix This PR fixes a bug. sig/planner SIG: Planner labels Sep 5, 2019

francis0407 requested review from zz-jason, winoros and lzmhhh123 September 5, 2019 05:08

francis0407 added the status/all tests passed label Sep 5, 2019

winoros reviewed Sep 5, 2019

View reviewed changes

remove ignore case

025f731

alivxxx reviewed Sep 5, 2019

View reviewed changes

francis0407 added 3 commits September 5, 2019 14:00

consider comment style hint

7245bb8

fix code style

791a744

fix code style

021a64b

winoros reviewed Sep 5, 2019

View reviewed changes

address comment

00f8952

Merge branch 'master' into use_index

bc06b14

francis0407 requested review from alivxxx and winoros September 5, 2019 08:10

zz-jason reviewed Sep 5, 2019

View reviewed changes

francis0407 added 3 commits September 5, 2019 23:06

address comment

c64959b

Merge branch 'use_index' of https://github.com/francis0407/tidb into …

0fc9a6c

…use_index

address comment

466d0d5

francis0407 added 3 commits September 5, 2019 23:13

minor change

034bd0e

fix fmt

8cfdd0c

Merge branch 'master' into use_index

cca636e

alivxxx reviewed Sep 6, 2019

View reviewed changes

francis0407 added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 6, 2019

lzmhhh123 approved these changes Sep 6, 2019

View reviewed changes

lzmhhh123 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 Sep 6, 2019

Merge branch 'master' into use_index

3b9db23

sre-bot merged commit d2e8bc6 into pingcap:master Sep 6, 2019

francis0407 mentioned this pull request Sep 10, 2019

planner: choose TableScan when use an empty index hint #12100

Merged

francis0407 mentioned this pull request Sep 11, 2019

planner: choose TableScan when use an empty index hint (#12037) #12154

Merged

zz-jason pushed a commit that referenced this pull request Sep 11, 2019

planner: choose TableScan when use an empty index hint (#12037) (#12154)

683540e

foreyes pushed a commit to foreyes/tidb that referenced this pull request Sep 16, 2019

planner: choose TableScan when use an empty index hint (pingcap#12037)

57c8267

foreyes mentioned this pull request Sep 16, 2019

planner: cherry-pick sql hint related PRs into release-3.1 #12192

Merged

francis0407 deleted the use_index branch October 29, 2019 02:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: choose TableScan when use an empty index hint #12037

planner: choose TableScan when use an empty index hint #12037

francis0407 commented Sep 5, 2019

codecov bot commented Sep 5, 2019 •

edited

Loading

winoros Sep 5, 2019

francis0407 Sep 5, 2019

francis0407 Sep 5, 2019

alivxxx Sep 5, 2019

francis0407 Sep 5, 2019

winoros Sep 5, 2019

francis0407 Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

zz-jason Sep 5, 2019

francis0407 Sep 5, 2019

alivxxx left a comment

lzmhhh123 left a comment

sre-bot commented Sep 6, 2019

planner: choose TableScan when use an empty index hint #12037

planner: choose TableScan when use an empty index hint #12037

Conversation

francis0407 commented Sep 5, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Sep 5, 2019 • edited Loading

Codecov Report

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

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

francis0407 commented Sep 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alivxxx left a comment

Choose a reason for hiding this comment

lzmhhh123 left a comment

Choose a reason for hiding this comment

sre-bot commented Sep 6, 2019

codecov bot commented Sep 5, 2019 •

edited

Loading