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

planner: do not convert filter to range for table scan with tiflash #12682

Merged

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

As the title says.

What is changed and how it works?

Change the range and the filter conditions in DataSource.getOriginalPhysicalTableScan.

Check List

Tests

  • Unit test

Side effects

  • Increased code complexity
  • Breaking backward compatibility

@lzmhhh123 lzmhhh123 added the sig/planner SIG: Planner label Oct 14, 2019
@zyxbest
Copy link
Contributor

zyxbest commented Oct 14, 2019

/run-unit-test

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #12682 into master will decrease coverage by 0.064%.
The diff coverage is 0%.

@@               Coverage Diff                @@
##             master     #12682        +/-   ##
================================================
- Coverage   79.9084%   79.8443%   -0.0641%     
================================================
  Files           461        461                
  Lines        104168     103678       -490     
================================================
- Hits          83239      82781       -458     
+ Misses        14819      14811         -8     
+ Partials       6110       6086        -24

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
but any bench result?

@lzmhhh123
Copy link
Contributor Author

@winoros The coprocessor in TiFlash can't support range scan. So they now always use the full range, the bench test will be added after the integrated test with them.

@zz-jason zz-jason requested a review from a team October 14, 2019 07:27
@ghost ghost requested review from qw4990 and SunRunAway and removed request for a team October 14, 2019 07:27
@winoros
Copy link
Member

winoros commented Oct 14, 2019

Ok

@winoros winoros added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 14, 2019
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

@alivxxx alivxxx added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 14, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 14, 2019

Your auto merge job has been accepted, waiting for 12676, 12640

@sre-bot
Copy link
Contributor

sre-bot commented Oct 14, 2019

/run-all-tests

@sre-bot sre-bot merged commit 63d1b1d into pingcap:master Oct 14, 2019
@lzmhhh123 lzmhhh123 deleted the dev/do_not_build_range_scan_for_tiflash branch October 14, 2019 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants