*: support IF EXISTS/IF NOT EXISTS for some DDLs #10723

csuzhangxc · 2019-06-05T07:48:11Z

What problem does this PR solve?

update parser to include pingcap/parser#337, and make some IF EXISTS/IF NOT EXISTS clauses work.

What is changed and how it works?

check IF EXISTS for
1. DROP COLUMN
2. CHANGE COLUMN
3. MODIFY COLUMN
4. DROP INDEX
5. DROP PARTITION
add IF NOT EXISTS for
1. ADD COLUMN
2. ADD INDEX
3. CREATE INDEX
4. ADD PARTITION

Check List

Tests

Unit test

Code changes

Has exported function/method change
Has interface methods change

…ARTITION` support `IF EXISTS`

CLAassistant · 2019-06-05T07:48:19Z

All committers have signed the CLA.

csuzhangxc · 2019-06-05T08:42:30Z

@winkyao @tiancaiamao PTAL

tiancaiamao

Maybe we have to check the EXISTS/NOT EXISTS both in ddl_api.go and ddl.go, in the DDL owner.
If we just check in the ddl_api.go, the information may be stale.

ddl/db_change_test.go

ddl/db_test.go

tiancaiamao · 2019-06-10T03:50:07Z

CI fail @csuzhangxc

csuzhangxc · 2019-06-12T08:07:14Z

Maybe we have to check the EXISTS/NOT EXISTS both in ddl_api.go and ddl.go, in the DDL owner.
If we just check in the ddl_api.go, the information may be stale.

@tiancaiamao DDL owner already checked exists/not exists before this PR, and it will return an error if possible, in this PR, we handled the returned err of doDDLJob. I think it's enough, right?

csuzhangxc · 2019-06-12T08:08:25Z

CI fail @csuzhangxc

@tiancaiamao I'll fix CI (because tidy failed) after pingcap/parser#337 merged.

ddl/ddl_api.go

ddl/ddl.go

ddl/ddl_api.go

codecov · 2019-06-13T11:03:11Z

Codecov Report

Merging #10723 into master will decrease coverage by 0.1042%.
The diff coverage is 88.6075%.

@@               Coverage Diff               @@
##            master     #10723        +/-   ##
===============================================
- Coverage   80.854%   80.7498%   -0.1043%     
===============================================
  Files          420        420                
  Lines        89147      89215        +68     
===============================================
- Hits         72079      72041        -38     
- Misses       11822      11948       +126     
+ Partials      5246       5226        -20

…ists # Conflicts: # go.mod # go.sum

csuzhangxc · 2019-06-13T11:26:24Z

/run-all-tests

tiancaiamao · 2019-06-13T12:42:41Z

LGTM

csuzhangxc · 2019-06-13T13:02:13Z

@winkyao @crazycs520 PTAL

XuHuaiyu · 2019-06-18T03:20:58Z

ddl/ddl_api.go

@@ -3023,6 +3084,10 @@ func (d *ddl) CreateIndex(ctx sessionctx.Context, ti ast.Ident, unique bool, ind
 	}

 	err = d.doDDLJob(ctx, job)
+	// key exists, but if_not_exists flags is true, so we ignore this error.
+	if ErrDupKeyName.Equal(err) && ifNotExists {


Do we need to report the warning here?

done in 0d609e3.

XuHuaiyu · 2019-06-18T03:21:50Z

ddl/ddl_api.go

@@ -3132,6 +3202,10 @@ func (d *ddl) DropIndex(ctx sessionctx.Context, ti ast.Ident, indexName model.CI
 	}

 	err = d.doDDLJob(ctx, job)
+	// index not exists, but if_exists flags is true, so we ignore this error.
+	if ErrCantDropFieldOrKey.Equal(err) && ifExists {


report warning here?

done in 0d609e3.

ddl/ddl_api.go

XuHuaiyu · 2019-06-18T03:23:37Z

ddl/db_test.go

+	// CREATE INDEX
+	sql = "create index idx_b on t1 (b)"
+	assertErrorCode(c, s.tk, sql, tmysql.ErrDupKeyName)
+	s.mustExec(c, "create index if not exists idx_b on t1 (b)")


Check the result of show warnings?

done in 2769cf1.

…ists

XuHuaiyu

LGTM

tiancaiamao · 2019-06-24T11:05:07Z

Please fix conflicts @csuzhangxc

tiancaiamao · 2019-06-24T11:23:54Z

/run-all-tests

tiancaiamao · 2019-06-24T11:40:17Z

/run-unit-test

csuzhangxc added 4 commits June 5, 2019 11:23

*: ADD COLUMN/ADD INDEX/CREATE INDEX support IF NOT EXISTS

1b43d71

*: ADD PARTITION support IF NOT EXISTS

80ae46d

*: DROP COLUMN/CHANGE COLUMN/MODIFY COLUMN/DROP INDEX/`DROP P…

9c74398

…ARTITION` support `IF EXISTS`

ddl: refine code

b55ec7f

csuzhangxc added status/DNM component/parser require-LGT3 Indicates that the PR requires three LGTM. type/new-feature labels Jun 5, 2019

csuzhangxc mentioned this pull request Jun 5, 2019

*: add IF EXISTS and IF NOT EXISTS supported for MariaDB pingcap/parser#337

Merged

tiancaiamao reviewed Jun 10, 2019

View reviewed changes

ddl/db_change_test.go Show resolved Hide resolved

ddl/db_test.go Show resolved Hide resolved

tiancaiamao reviewed Jun 13, 2019

View reviewed changes

ddl/ddl_api.go Show resolved Hide resolved

ddl/ddl_api.go Show resolved Hide resolved

ddl/ddl.go Show resolved Hide resolved

ddl/ddl_api.go Show resolved Hide resolved

ddl/ddl_api.go Show resolved Hide resolved

*: update parser

6074922

csuzhangxc added 3 commits June 13, 2019 19:14

Merge remote-tracking branch 'remotes/origin/master' into alter-if-ex…

c54a966

…ists # Conflicts: # go.mod # go.sum

*: revert replace in go.mod

442dfce

*: go mod tidy

e2fedb5

winkyao removed the require-LGT3 Indicates that the PR requires three LGTM. label Jun 13, 2019

csuzhangxc removed the status/DNM label Jun 13, 2019

tiancaiamao added status/LGT1 Indicates that a PR has LGTM 1. status/all tests passed labels Jun 13, 2019

XuHuaiyu reviewed Jun 18, 2019

View reviewed changes

csuzhangxc added 3 commits June 18, 2019 11:50

Merge remote-tracking branch 'remotes/origin/master' into alter-if-ex…

0748603

…ists

ddl: report the warning

0d609e3

ddl: check warning

2769cf1

XuHuaiyu reviewed Jun 18, 2019

View reviewed changes

csuzhangxc added 2 commits June 24, 2019 19:14

*: resolve conflicts

98191d3

*: resolve conflicts

91aef43

tiancaiamao approved these changes Jun 24, 2019

View reviewed changes

tiancaiamao merged commit a935d93 into pingcap:master Jun 24, 2019

csuzhangxc deleted the alter-if-exists branch June 24, 2019 11:48

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support IF EXISTS/IF NOT EXISTS for some DDLs #10723

*: support IF EXISTS/IF NOT EXISTS for some DDLs #10723

csuzhangxc commented Jun 5, 2019 •

edited

Loading

CLAassistant commented Jun 5, 2019 •

edited

Loading

csuzhangxc commented Jun 5, 2019

tiancaiamao left a comment

tiancaiamao commented Jun 10, 2019

csuzhangxc commented Jun 12, 2019

csuzhangxc commented Jun 12, 2019

codecov bot commented Jun 13, 2019 •

edited

Loading

csuzhangxc commented Jun 13, 2019

tiancaiamao commented Jun 13, 2019

csuzhangxc commented Jun 13, 2019

XuHuaiyu Jun 18, 2019

csuzhangxc Jun 18, 2019

XuHuaiyu Jun 18, 2019

csuzhangxc Jun 18, 2019

XuHuaiyu Jun 18, 2019

csuzhangxc Jun 18, 2019

XuHuaiyu left a comment

tiancaiamao commented Jun 24, 2019

tiancaiamao commented Jun 24, 2019

tiancaiamao commented Jun 24, 2019

*: support IF EXISTS/IF NOT EXISTS for some DDLs #10723

*: support IF EXISTS/IF NOT EXISTS for some DDLs #10723

Conversation

csuzhangxc commented Jun 5, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

CLAassistant commented Jun 5, 2019 • edited Loading

csuzhangxc commented Jun 5, 2019

tiancaiamao left a comment

Choose a reason for hiding this comment

tiancaiamao commented Jun 10, 2019

csuzhangxc commented Jun 12, 2019

csuzhangxc commented Jun 12, 2019

codecov bot commented Jun 13, 2019 • edited Loading

Codecov Report

csuzhangxc commented Jun 13, 2019

tiancaiamao commented Jun 13, 2019

csuzhangxc commented Jun 13, 2019

XuHuaiyu Jun 18, 2019

Choose a reason for hiding this comment

csuzhangxc Jun 18, 2019

Choose a reason for hiding this comment

XuHuaiyu Jun 18, 2019

Choose a reason for hiding this comment

csuzhangxc Jun 18, 2019

Choose a reason for hiding this comment

XuHuaiyu Jun 18, 2019

Choose a reason for hiding this comment

csuzhangxc Jun 18, 2019

Choose a reason for hiding this comment

XuHuaiyu left a comment

Choose a reason for hiding this comment

tiancaiamao commented Jun 24, 2019

tiancaiamao commented Jun 24, 2019

tiancaiamao commented Jun 24, 2019

csuzhangxc commented Jun 5, 2019 •

edited

Loading

CLAassistant commented Jun 5, 2019 •

edited

Loading

codecov bot commented Jun 13, 2019 •

edited

Loading