-
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
ddl: split partition region when add a new partition #16537
Conversation
Signed-off-by: crazycs520 <crazycs520@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #16537 +/- ##
================================================
- Coverage 80.4155% 80.3685% -0.0471%
================================================
Files 507 507
Lines 137303 137127 -176
================================================
- Hits 110413 110207 -206
- Misses 18273 18301 +28
- Partials 8617 8619 +2 |
ddl/ddl_api.go
Outdated
@@ -1625,7 +1625,9 @@ func (d *ddl) preSplitAndScatter(ctx sessionctx.Context, tbInfo *model.TableInfo | |||
} else { | |||
scatterRegion = variable.TiDBOptOn(val) | |||
} | |||
pi := tbInfo.GetPartitionInfo() | |||
if pi == nil { | |||
pi = tbInfo.GetPartitionInfo() |
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 makes me confused.
What's the difference between argument pi
and tbInfo.GetPartitionInfo()
, are't they the same thing?
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.
pi
will be the added partition information when AddTablePartitions
call this function.
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.
Please add comment for this parameter, otherwise when somebody review this code later in the future, he will have the same question.
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.
If AddTablePartitions
calls this function and fill with partInfo
, pi
isn't nil, so why we need to add the check of if pi == nil
?
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.
CreateTableWithInfo
will call this function too, then pi
will be nil.
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.
Could we call d.preSplitAndScatter(ctx, tbInfo, tbInfo.GetPartitionInfo())
to handle it in CreateTableWithInfo
?
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.
Rest LGTM
ddl/ddl_api.go
Outdated
@@ -1625,7 +1625,9 @@ func (d *ddl) preSplitAndScatter(ctx sessionctx.Context, tbInfo *model.TableInfo | |||
} else { | |||
scatterRegion = variable.TiDBOptOn(val) | |||
} | |||
pi := tbInfo.GetPartitionInfo() | |||
if pi == nil { | |||
pi = tbInfo.GetPartitionInfo() |
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.
Please add comment for this parameter, otherwise when somebody review this code later in the future, he will have the same question.
Signed-off-by: crazycs520 <crazycs520@gmail.com>
LGTM |
ref #14478 |
Signed-off-by: crazycs520 <crazycs520@gmail.com>
Signed-off-by: crazycs520 <crazycs520@gmail.com>
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
/run-all-tests |
/run-cherry-picker |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.0 in PR #17663 |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-3.1 in PR #17664 |
Signed-off-by: sre-bot <sre-bot@pingcap.com>
cherry pick to release-4.0 in PR #17665 |
Signed-off-by: crazycs520 crazycs520@gmail.com
What problem does this PR solve?
Split new region when add partitions.
before
This PR
What is changed and how it works?
Proposal: xxx
What's Changed:
How it Works:
Related changes
Check List
Tests
Side effects
Release note