ddl: support alter table attributes to add a label rule #26511

rleungx · 2021-07-23T08:48:43Z

What problem does this PR solve?

This PR is going to support

ALTER TABLE t ATTRIBUTES="xxx"

to add an attribute for a table.

What is changed and how it works?

What's Changed:

add a new type AlterTableAttributes in AlterTable.
introduce a new struct Rule which is the same as LabelRule defined in PD
support synchronizing a new rule to PD.

Check List

Tests

Unit test (need more unit tests)

Documentation

Release note

None

ti-chi-bot · 2021-07-23T08:48:44Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Yisaer
xhebox

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ddl/label/attributes.go

ddl/ddl_api.go

ddl/label/attributes_test.go

xhebox · 2021-07-23T09:59:47Z

ddl/label/rule.go

+
+// ResetTable will reset the label rule for a table with a given ID and names.
+func (r *Rule) ResetTable(id int64, dbName, tableName string) *Rule {
+	r.ID = fmt.Sprintf(TableIDFormat, IDPrefix, dbName, tableName)


Note that text ID is troublesome for rename table, and flashback/recover where you only get the integer. You will need history job and model out of ddl package to retrieve the name.
.
Check #20575 for gc and placement rules.

Yes. And I have worked on it in the last few days.

ddl/label/rule.go

ddl/label/attributes.go

ddl/label/rule.go

nolouch · 2021-07-28T03:33:33Z

PTAL @JmPotato

JmPotato

Rest LGTM.

ddl/label/attributes.go

domain/infosync/info.go

rleungx · 2021-07-28T09:17:44Z

/run-check_dev_2

ti-chi-bot · 2021-07-28T11:07:29Z

@JmPotato: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ddl/ddl_api.go

nolouch · 2021-07-28T18:19:37Z

ddl/label/attributes.go

+func NewLabels(attrs []string) Labels {
+	labels := make(Labels, 0, len(attrs))
+	for _, attr := range attrs {
+		label := NewLabel(attr)


should valid the attributes? only supports nomerge at present.

nolouch · 2021-07-30T03:55:24Z

/run-check_dev_2

Yisaer

lgtm

Yisaer · 2021-07-30T06:00:01Z

/merge

ti-chi-bot · 2021-07-30T06:00:05Z

This pull request has been accepted and is ready to merge.

Commit hash: 6ab0df62efcbad72a20335afc80bb0de1f94de8f

Signed-off-by: Ryan Leung <[email protected]>

Yisaer · 2021-07-30T08:10:09Z

/merge

ti-chi-bot · 2021-07-30T08:10:13Z

This pull request has been accepted and is ready to merge.

Commit hash: 7e20f79

rleungx · 2021-07-30T08:29:11Z

/run-unit-test

rleungx · 2021-07-30T08:36:40Z

/run-unit-test

ti-chi-bot · 2021-07-30T08:59:16Z

@rleungx: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

rleungx · 2021-07-30T09:11:24Z

/run-check_dev_2

ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 23, 2021

rleungx force-pushed the alter-table-attribute branch from 48263c2 to dc5941c Compare July 23, 2021 09:05

github-actions bot added the sig/sql-infra SIG: SQL Infra label Jul 23, 2021

nolouch requested review from nolouch, Yisaer and xhebox July 23, 2021 09:32

xhebox reviewed Jul 23, 2021

View reviewed changes

rleungx force-pushed the alter-table-attribute branch 2 times, most recently from baf2d78 to ad5bcee Compare July 27, 2021 03:17

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 27, 2021

rleungx mentioned this pull request Jul 27, 2021

ddl: support drop table for label rule #26630

Merged

6 tasks

JmPotato reviewed Jul 28, 2021

View reviewed changes

ddl/label/attributes.go Outdated Show resolved Hide resolved

ddl/label/attributes.go Outdated Show resolved Hide resolved

domain/infosync/info.go Show resolved Hide resolved

ti-chi-bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Jul 28, 2021

rleungx force-pushed the alter-table-attribute branch 5 times, most recently from bdae5e5 to 5c1e5ad Compare July 28, 2021 08:20

rleungx requested a review from xhebox July 28, 2021 08:25

github-actions bot added the sig/execution SIG execution label Jul 28, 2021

JmPotato approved these changes Jul 28, 2021

View reviewed changes

nolouch reviewed Jul 28, 2021

View reviewed changes

rleungx force-pushed the alter-table-attribute branch from af5bf87 to 3f9b5c8 Compare July 29, 2021 02:25

xhebox approved these changes Jul 29, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 29, 2021

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2021

Yisaer approved these changes Jul 30, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 30, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 30, 2021

rleungx added 7 commits July 30, 2021 14:55

support alter table attributes to add a label rule

de53021

Signed-off-by: Ryan Leung <[email protected]>

address comments

4e87c81

Signed-off-by: Ryan Leung <[email protected]>

add some rule tests

57804a9

Signed-off-by: Ryan Leung <[email protected]>

update the dependency

c06210c

Signed-off-by: Ryan Leung <[email protected]>

add more tests

be3e063

Signed-off-by: Ryan Leung <[email protected]>

address the comment

e31dbab

Signed-off-by: Ryan Leung <[email protected]>

downgrade errors dependency

7e20f79

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the alter-table-attribute branch from abeeec8 to 7e20f79 Compare July 30, 2021 07:47

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 30, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 30, 2021

Merge branch 'master' into alter-table-attribute

cddbad6

Merge branch 'master' into alter-table-attribute

ea34924

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 30, 2021

ti-chi-bot merged commit 17523d3 into pingcap:master Jul 30, 2021

morgo mentioned this pull request Aug 1, 2021

attributes is now a reserved word #26778

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: support alter table attributes to add a label rule #26511

ddl: support alter table attributes to add a label rule #26511

rleungx commented Jul 23, 2021 •

edited

Loading

ti-chi-bot commented Jul 23, 2021 •

edited

Loading

xhebox Jul 23, 2021

rleungx Jul 23, 2021

nolouch commented Jul 28, 2021

JmPotato left a comment

rleungx commented Jul 28, 2021

ti-chi-bot commented Jul 28, 2021

nolouch Jul 28, 2021

nolouch commented Jul 30, 2021

Yisaer left a comment

Yisaer commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

Yisaer commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

rleungx commented Jul 30, 2021

rleungx commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

rleungx commented Jul 30, 2021

ddl: support alter table attributes to add a label rule #26511

ddl: support alter table attributes to add a label rule #26511

Conversation

rleungx commented Jul 23, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jul 23, 2021 • edited Loading

xhebox Jul 23, 2021

Choose a reason for hiding this comment

rleungx Jul 23, 2021

Choose a reason for hiding this comment

nolouch commented Jul 28, 2021

JmPotato left a comment

Choose a reason for hiding this comment

rleungx commented Jul 28, 2021

ti-chi-bot commented Jul 28, 2021

nolouch Jul 28, 2021

Choose a reason for hiding this comment

nolouch commented Jul 30, 2021

Yisaer left a comment

Choose a reason for hiding this comment

Yisaer commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

Yisaer commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

rleungx commented Jul 30, 2021

rleungx commented Jul 30, 2021

ti-chi-bot commented Jul 30, 2021

rleungx commented Jul 30, 2021

rleungx commented Jul 23, 2021 •

edited

Loading

ti-chi-bot commented Jul 23, 2021 •

edited

Loading