-
Notifications
You must be signed in to change notification settings - Fork 101
Conversation
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #137 +/- ##
=========================================
Coverage ? 72.95%
=========================================
Files ? 33
Lines ? 3257
Branches ? 0
=========================================
Hits ? 2376
Misses ? 591
Partials ? 290
Continue to review full report at Codecov.
|
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
Signed-off-by: 5kbpers <[email protected]>
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
// Sort the tables by id for ensuring the new tables has same id ordering as the old tables. | ||
// We require this constrain since newTableID of tableID+1 must be not bigger | ||
// than newTableID of tableID. | ||
sort.Sort(utils.Tables(tables)) |
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.
Can we remove utils.Tables
?
rewriteRules.Table = append(rewriteRules.Table, rules.Table...) | ||
rewriteRules.Data = append(rewriteRules.Data, rules.Data...) | ||
newTables = append(newTables, newTableInfo) | ||
} | ||
// If tableID + 1 has already exist, then we don't need to add a new rewrite rule for it. | ||
for oldID, newID := range tableIDMap { |
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.
We should update validate.go since we change the rewrite process.
Signed-off-by: 5kbpers <[email protected]>
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
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
Support table partition: