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/core: fix partition selection on PointGet/BatchPointGet (#19146) #19164

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Issue Number: Fix #19141 on the master branch

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

Cherry pick #19146 to master, from 4.0

Check List

Tests

  • Unit test

Release note

  • Fix bug PointGet and BatchPointGet do not consider partition selection grammar and get incorrect results.

…cap#19146)

* planner/core: fix partition selection on PointGet/BatchPointGet

* make golint happy
@tiancaiamao tiancaiamao requested a review from a team as a code owner August 13, 2020 02:49
@tiancaiamao tiancaiamao requested review from eurekaka, SunRunAway and imtbkcat and removed request for a team and eurekaka August 13, 2020 02:49
@tiancaiamao tiancaiamao added the sig/planner SIG: Planner label Aug 13, 2020
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

@lysu,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: planner(slack).

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao
Copy link
Contributor Author

/run-integration-ddl-test

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 18, 2020
@tiancaiamao
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

@tiancaiamao Oops! auto merge is restricted to Committers of the SIG.See the corresponding SIG page for more information. Related SIGs: planner(slack).

@imtbkcat imtbkcat added sig/transaction SIG:Transaction and removed sig/planner SIG: Planner labels Aug 18, 2020
Copy link

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 18, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 18, 2020
@imtbkcat
Copy link

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 18, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@tiancaiamao merge failed.

@tiancaiamao
Copy link
Contributor Author

/run-unit-test

@tiancaiamao
Copy link
Contributor Author

/run-check_dev_2

@tiancaiamao tiancaiamao merged commit e196a3e into pingcap:master Aug 18, 2020
@tiancaiamao tiancaiamao deleted the point-get-partition branch August 18, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/transaction SIG:Transaction 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.

PointGet/BatchPointGet will break partition selection
5 participants