-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
planner/core: fix partition selection on PointGet/BatchPointGet #19146
Conversation
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -749,6 +764,16 @@ func tryPointGetPlan(ctx sessionctx.Context, selStmt *ast.SelectStmt) *PointGetP | |||
return nil | |||
} | |||
|
|||
func partitionNameInSet(name model.CIStr, pnames []model.CIStr) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a performance issue when the partition count is large, For example: 65535.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name slice is not all the partitions of a table.
It's just the selected ones:
select * from t partition (p0,p1,p2) ...
A sane person would not write a SQL like this:
select * from t partition (p0,p1,p2....p65535) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -429,3 +429,22 @@ func (s *testPointGetSuite) TestBatchPointGetPartition(c *C) { | |||
tk.MustExec("delete from t where (a,b) in ((1,1),(2,2),(3,3),(4,4))") | |||
tk.MustQuery("select * from t where (a, b) in ((1, 1), (2, 2), (3, 3), (4, 4))").Check(testkit.Rows()) | |||
} | |||
|
|||
func (s *testPointGetSuite) TestIssue19141(c *C) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this case be ported back to master branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
/merge |
Sorry @SunRunAway, you don't have permission to trigger auto merge event on this branch. |
/merge |
/run-all-tests |
…cap#19146) * planner/core: fix partition selection on PointGet/BatchPointGet * make golint happy
What problem does this PR solve?
Issue Number: close #19141
Problem Summary:
PointGet/BatchPointGet on the hash partition table does not take partition selection grammar into consideration.
What is changed and how it works?
What's Changed:
How it Works:
Related changes
This PR is on the 4.0 branch, I will file another one to fix the master.
Check List
Tests
Release note