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

ranger: some code clean up #8663

Merged
merged 5 commits into from
Dec 13, 2018
Merged

ranger: some code clean up #8663

merged 5 commits into from
Dec 13, 2018

Conversation

winoros
Copy link
Member

@winoros winoros commented Dec 12, 2018

What problem does this PR solve?

The deleted code actually does not always take effect on the same range query.

mysql> create table t(a varchar(50), b int, index idx(a, b));
Query OK, 0 rows affected (0.01 sec)

mysql> explain select * from t where a > 'abc';
+-------------------+---------+------+-----------------------------------------------------------------------------------+
| id                | count   | task | operator info                                                                     |
+-------------------+---------+------+-----------------------------------------------------------------------------------+
| IndexReader_9     | 3333.33 | root | index:IndexScan_8                                                                 |
| └─IndexScan_8     | 3333.33 | cop  | table:t, index:a, b, range:("abc" +inf,+inf +inf], keep order:false, stats:pseudo |
+-------------------+---------+------+-----------------------------------------------------------------------------------+
2 rows in set (0.00 sec)

mysql> explain select a from t where a > 'abc';
+-------------------+---------+------+-------------------------------------------------------------------------+
| id                | count   | task | operator info                                                           |
+-------------------+---------+------+-------------------------------------------------------------------------+
| IndexReader_9     | 3333.33 | root | index:IndexScan_8                                                       |
| └─IndexScan_8     | 3333.33 | cop  | table:t, index:a, b, range:("abc",+inf], keep order:false, stats:pseudo |
+-------------------+---------+------+-------------------------------------------------------------------------+

What is changed and how it works?

Remove these codes won't make the result wrong. PrefixNext can promise correctness when we convert it to kvRange.

// PrefixNext returns the next prefix key.
//
// Assume there are keys like:
//
//   rowkey1
//   rowkey1_column1
//   rowkey1_column2
//   rowKey2
//
// If we seek 'rowkey1' Next, we will get 'rowkey1_column1'.
// If we seek 'rowkey1' PrefixNext, we will get 'rowkey2'.

This can help to simplify #8471 logic

Check List

Tests

  • Unit test
  • Integration test

This change is Reviewable

@winoros winoros added the sig/planner SIG: Planner label Dec 12, 2018
@winoros winoros changed the title Ranger clean up ranger: some code clean up Dec 12, 2018
@ngaut
Copy link
Member

ngaut commented Dec 12, 2018

/run-all-tests

Copy link
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
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 type/enhancement The issue or PR belongs to an enhancement. status/LGT2 Indicates that a PR has LGTM 2. status/all tests passed labels Dec 13, 2018
@zz-jason zz-jason merged commit 7af338c into pingcap:master Dec 13, 2018
iamzhoug37 pushed a commit to iamzhoug37/tidb that referenced this pull request Dec 13, 2018
@winoros winoros deleted the ranger-clean-up branch December 19, 2018 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants