ddl: Support multiple table rename #19962

TszKitLo40 · 2020-09-12T03:42:09Z

What problem does this PR solve?

Issue Number: close #9384
Related to PR: pingcap/parser#1021

Problem Summary:

What is changed and how it works?

What's Changed:
Support multiple table rename
RENAME TABLE tbl_name TO new_tbl_name [, tbl_name2 TO new_tbl_name2] ...

Check List

Tests

Unit test

Release note

Support multiple table rename

ddl/db_test.go

zimulala · 2020-09-17T15:00:32Z

ddl/ddl_api.go

+		SchemaName: newSchema.Name.L,
+		Type:       model.ActionRenameTable,
+		BinlogInfo: &model.HistoryInfo{},
+		Args:       []interface{}{oldSchemaIDs, tableNames, tableIDs},


Suggested change

Args: []interface{}{oldSchemaIDs, tableNames, tableIDs},

Args: []interface{}{oldSchemaIDs, newSchemaIDs, tableNames, tableIDs},

ddl/ddl_worker.go

zimulala · 2020-09-17T15:04:08Z

ddl/ddl_api.go

+				fmt.Sprintf("%s.%s", oldIdents[i].Schema, oldIdents[i].Name),
+				fmt.Sprintf("%s.%s", newIdents[i].Schema, newIdents[i].Name),
+				168,
+				fmt.Sprintf("Database `%s` doesn't exist", newIdents[i].Schema))


Could we extract line4359-line4386 into a function? Then, both RenameTable and RenameTables can use this function.

zimulala · 2020-09-17T15:05:36Z

ddl/table.go

+		ver, err := updateSchemaVersion(t, job)
+		if err != nil {
+			return ver, errors.Trace(err)
+		}


Ditto. Could we extract a function to reduce duplicate code？

…zKitLo40/tidb into support-multiple-table-rename

djshow832 · 2020-09-22T02:46:43Z

ddl/db_test.go

+	tk.MustExec("create database rename1")
+	tk.MustExec("create database rename2")
+	tk.MustExec("create database rename3")
+	tk.MustExec("create database rename4")
+	tk.MustExec("create table rename1.t1 (a int primary key auto_increment)")
+	tk.MustExec("create table rename3.t3 (a int primary key auto_increment)")
+	tk.MustExec("rename table rename1.t1 to rename2.t2, rename3.t3 to rename4.t4")
+	// Make sure the drop old database doesn't affect the rename2.t2, rename4.t4's operations.
+	tk.MustExec("drop database rename1")
+	tk.MustExec("drop database rename3")
+	tk.MustExec("insert rename2.t2 values ()")
+	tk.MustExec("insert rename4.t4 values ()")
+	tk.MustQuery("select * from rename2.t2").Check(testkit.Rows("1"))
+	tk.MustQuery("select * from rename4.t4").Check(testkit.Rows("1"))
+	tk.MustExec("drop database rename2")
+	tk.MustExec("drop database rename4")


I think this part is duplicate with the following part. It can be removed.
However, more cases are needed.
E.g.

create table rename1.t1 (a int primary key auto_increment); create table rename3.t3 (a int primary key auto_increment); rename table rename1.t1 to rename2.t2, rename3.t3 to rename2.t2; # fail rename table rename1.t1 to rename2.t2, rename2.t2 to rename1.t1; # succeed

ddl/ddl_api.go

AilinKid

LGTM
ps: remember to add ActionRenameTables to cancel logic in rollingback.go

TszKitLo40 · 2020-10-27T11:01:42Z

LGTM
ps: remember to add ActionRenameTables to cancel logic in rollingback.go

Do you mean we should add ActionRenameTables in the L444 in rollingback.go?

ti-challenge-bot · 2020-10-27T11:01:47Z

There is no reward for this challenge pull request, so you can request a reward from @zimulala.

Details

Tip : About reward you can refs to reward-command.

Warning: None

AilinKid · 2020-10-27T12:53:59Z

LGTM
ps: remember to add ActionRenameTables to cancel logic in rollingback.go

Do you mean we should add ActionRenameTables in the L444 in rollingback.go?

yes, just like model.ActionRenameTable

ti-challenge-bot · 2020-10-27T12:54:04Z

There is no reward for this challenge pull request, so you can request a reward from @zimulala.

Details

Tip : About reward you can refs to reward-command.

Warning: None

…zKitLo40/tidb into support-multiple-table-rename

TszKitLo40 · 2020-10-27T13:00:06Z

LGTM
ps: remember to add ActionRenameTables to cancel logic in rollingback.go

Do you mean we should add ActionRenameTables in the L444 in rollingback.go?

yes, just like model.ActionRenameTable

I have added it.

ti-challenge-bot · 2020-10-27T13:00:11Z

There is no reward for this challenge pull request, so you can request a reward from @zimulala.

Details

Tip : About reward you can refs to reward-command.

Warning: None

AilinKid · 2020-10-27T13:02:26Z

good, you can make tidy to resolve the go.sum's conflicts

djshow832 · 2020-10-28T03:26:15Z

/merge

ti-srebot · 2020-10-28T03:26:49Z

/run-all-tests

Rustin170506 · 2020-10-28T06:34:52Z

@djshow832 Please don't merge PR when the CI still pending.

cc: @TszKitLo40 @zimulala

djshow832 · 2020-10-28T09:01:02Z

@zimulala is on leave and can't give the reward. @hi-rustin

djshow832 · 2020-10-28T09:01:30Z

/reward 1500

ti-challenge-bot · 2020-10-28T09:01:35Z

This PR already closed!

TszKitLo40 added 2 commits September 12, 2020 11:33

ddl: Support multiple table rename

d5c145b

Merge branch 'master' into support-multiple-table-rename

d4cf528

TszKitLo40 requested a review from a team as a code owner September 12, 2020 03:42

TszKitLo40 requested review from fzhedu and removed request for a team September 12, 2020 03:42

ti-srebot added the contribution This PR is from a community contributor. label Sep 12, 2020

github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution labels Sep 12, 2020

TszKitLo40 and others added 5 commits September 12, 2020 11:48

Fix unit test

38c0db1

Fix check_dev2

a2961ef

Make rename tables in the same job

8b127c1

Fix dev

a6249a5

Merge branch 'master' into support-multiple-table-rename

954572a

zimulala added high-performance challenge-program and removed sig/execution SIG execution labels Sep 17, 2020

github-actions bot added the sig/execution SIG execution label Sep 17, 2020

djshow832 requested review from zimulala and AilinKid September 17, 2020 13:26

zimulala reviewed Sep 17, 2020

View reviewed changes

ti-srebot added the difficulty/easy label Sep 18, 2020

TszKitLo40 added 6 commits September 18, 2020 14:45

Refactor the code

9618b2f

Add unit tests

43ec78c

Merge branch 'master' into support-multiple-table-rename

9c87f28

Merge branch 'support-multiple-table-rename' of https://github.com/Ts…

2e6154c

…zKitLo40/tidb into support-multiple-table-rename

Fix check_dev

c2b111f

Fix check_dev

eadd43e

djshow832 reviewed Sep 22, 2020

View reviewed changes

Add unit tests and use new version of paser

bcd48c5

ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 27, 2020

ti-srebot previously approved these changes Oct 27, 2020

View reviewed changes

bb7133 added the require-LGT3 Indicates that the PR requires three LGTM. label Oct 27, 2020

AilinKid reviewed Oct 27, 2020

View reviewed changes

ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Oct 27, 2020

TszKitLo40 added 2 commits October 27, 2020 20:57

Add rollback for renametables.

f226cab

Merge branch 'support-multiple-table-rename' of https://github.com/Ts…

40a72f5

…zKitLo40/tidb into support-multiple-table-rename

TszKitLo40 dismissed ti-srebot’s stale review via 40a72f5 October 27, 2020 12:59

TszKitLo40 added 3 commits October 28, 2020 10:24

Merge branch 'master' into support-multiple-table-rename

7f89473

fix check_dev

d41995b

Replace parser.

1bf151e

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 28, 2020

djshow832 approved these changes Oct 28, 2020

View reviewed changes

djshow832 merged commit 91db54e into pingcap:master Oct 28, 2020

XuHuaiyu removed the sig/execution SIG execution label Apr 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: Support multiple table rename #19962

ddl: Support multiple table rename #19962

TszKitLo40 commented Sep 12, 2020 •

edited

Loading

zimulala Sep 17, 2020

zimulala Sep 17, 2020

zimulala Sep 17, 2020

djshow832 Sep 22, 2020

AilinKid left a comment •

edited

Loading

TszKitLo40 commented Oct 27, 2020 •

edited

Loading

ti-challenge-bot bot commented Oct 27, 2020

AilinKid commented Oct 27, 2020

ti-challenge-bot bot commented Oct 27, 2020

TszKitLo40 commented Oct 27, 2020

ti-challenge-bot bot commented Oct 27, 2020

AilinKid commented Oct 27, 2020

djshow832 commented Oct 28, 2020

ti-srebot commented Oct 28, 2020

Rustin170506 commented Oct 28, 2020

djshow832 commented Oct 28, 2020

djshow832 commented Oct 28, 2020

ti-challenge-bot bot commented Oct 28, 2020

	Args: []interface{}{oldSchemaIDs, tableNames, tableIDs},
	Args: []interface{}{oldSchemaIDs, newSchemaIDs, tableNames, tableIDs},

ddl: Support multiple table rename #19962

ddl: Support multiple table rename #19962

Conversation

TszKitLo40 commented Sep 12, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

zimulala Sep 17, 2020

Choose a reason for hiding this comment

zimulala Sep 17, 2020

Choose a reason for hiding this comment

zimulala Sep 17, 2020

Choose a reason for hiding this comment

djshow832 Sep 22, 2020

Choose a reason for hiding this comment

AilinKid left a comment • edited Loading

Choose a reason for hiding this comment

TszKitLo40 commented Oct 27, 2020 • edited Loading

ti-challenge-bot bot commented Oct 27, 2020

AilinKid commented Oct 27, 2020

ti-challenge-bot bot commented Oct 27, 2020

TszKitLo40 commented Oct 27, 2020

ti-challenge-bot bot commented Oct 27, 2020

AilinKid commented Oct 27, 2020

djshow832 commented Oct 28, 2020

ti-srebot commented Oct 28, 2020

Rustin170506 commented Oct 28, 2020

djshow832 commented Oct 28, 2020

djshow832 commented Oct 28, 2020

ti-challenge-bot bot commented Oct 28, 2020

TszKitLo40 commented Sep 12, 2020 •

edited

Loading

AilinKid left a comment •

edited

Loading

TszKitLo40 commented Oct 27, 2020 •

edited

Loading