Skip to content

ddl, executor, server: add drop if exists warnings#14807

Merged
sre-bot merged 15 commits intopingcap:masterfrom
0xPoe:rustin-patch-drop
Feb 18, 2020
Merged

ddl, executor, server: add drop if exists warnings#14807
sre-bot merged 15 commits intopingcap:masterfrom
0xPoe:rustin-patch-drop

Conversation

@0xPoe
Copy link
Copy Markdown
Member

@0xPoe 0xPoe commented Feb 16, 2020 •
edited by zz-jason
Loading

Signed-off-by: Rustin-Liu rustin.liu@gmail.com

What problem does this PR solve?

close #7867

What is changed and how it works?

Add warnings.

Check List

Tests

  • Unit test

Code changes

None

Side effects

None
Related changes

None

Release note

None

@0xPoe
executor: add drop if exists warnings(close #7867)
c2e4e2b 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Feb 16, 2020
0xPoe added 3 commits February 16, 2020 11:17
@0xPoe
executor: fix ci broken
01a7881 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
Merge branch 'master' into rustin-patch-drop
363f8c8
@0xPoe
*: add more test cases and fix ci broken
2a704b3 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 16, 2020

@sre-bot /run-all-tests

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Feb 16, 2020

/run-all-tests

0xPoe
0xPoe commented Feb 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member Author

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selfcheck passed.

@0xPoe 0xPoe marked this pull request as ready for review February 16, 2020 03:56
bb7133
bb7133 reviewed Feb 16, 2020
View reviewed changes
Comment thread executor/ddl_test.go Outdated
0xPoe added 5 commits February 16, 2020 16:07
@0xPoe
ddl: move test case
525a69f 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
ddl: add drop if exists test case for view
ccd434b 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
ddl: add drop if exists test case for sequence
b845e1d 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
ddl: modify drop if exists test case for sequence
6dd0100 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
ddl: remove useless test case
b30ae36 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 16, 2020

@sre-bot /run-all-tests

bb7133
bb7133 reviewed Feb 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bb7133 bb7133 added this to the v4.0.0-beta.1 milestone Feb 16, 2020
@bb7133 bb7133 added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 16, 2020
@zz-jason zz-jason changed the title executor: add drop if exists warnings(close #7867) ddl, executor, server: add drop if exists warnings(close #7867) Feb 17, 2020
@zz-jason zz-jason changed the title ddl, executor, server: add drop if exists warnings(close #7867) ddl, executor, server: add drop if exists warnings Feb 17, 2020
zimulala
zimulala reviewed Feb 17, 2020
View reviewed changes
Comment thread executor/ddl_test.go Outdated
Comment thread executor/ddl_test.go Outdated
Comment thread ddl/sequence_test.go Outdated
@0xPoe
*: refine comment
7e79170 
Signed-off-by: Rustin-Liu <rustin.liu@gmail.com>
zimulala
zimulala reviewed Feb 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tangenta
Copy link
Copy Markdown
Contributor

tangenta commented Feb 17, 2020

@Rustin-Liu The changes in server/http_handler_test.go and server/server_test.go seem to be irrelavent.

Could you please explain why we can not do this change?

I'm just confused by this code.

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 17, 2020

@tangenta Because if the Strict if true, we will get err.

tidb/server/http_handler_test.go

Line 941 in 7e79170

dbt.mustExec("drop table if exists t2;")

tangenta
tangenta approved these changes Feb 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zimulala
Copy link
Copy Markdown
Contributor

zimulala commented Feb 17, 2020

/run-all-tests

@zimulala zimulala added status/LGT3 The PR has already had 3 LGTM. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Feb 17, 2020
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Feb 17, 2020

/run-all-tests

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Feb 17, 2020

@Rustin-Liu merge failed.

AilinKid
AilinKid reviewed Feb 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@AilinKid AilinKid left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't worry, it should fix some code in the tidb-test.

@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 17, 2020

/run-all-tests

@zimulala
Copy link
Copy Markdown
Contributor

zimulala commented Feb 17, 2020

@AilinKid is dealing with this problem.

@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 17, 2020

/run-common-test tidb-test=pr/990

@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 17, 2020

/run-integration-common-test tidb-test=pr/990

1 similar comment
@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 17, 2020

/run-integration-common-test tidb-test=pr/990

@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 17, 2020

/run-common-test tidb-test=pr/990

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 17, 2020

@sre-bot /merge

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 17, 2020

@sre-bot /run-auto-merge

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Feb 17, 2020

@Rustin-Liu No command or invalid command

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 17, 2020

@sre-bot You are stupid🙄

@AilinKid
Copy link
Copy Markdown
Contributor

AilinKid commented Feb 18, 2020

@sre-bot You are stupid🙄

don't worry about it, it should merge the tidb-test PR first then to it.

@0xPoe
Copy link
Copy Markdown
Member Author

0xPoe commented Feb 18, 2020

@sre-bot You are stupid🙄

don't worry about it, it should merge the tidb-test PR first then to it.

Got it. Thanks for your supporting!

@tangenta
Copy link
Copy Markdown
Contributor

tangenta commented Feb 18, 2020

/merge

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Feb 18, 2020

/run-all-tests

@sre-bot sre-bot merged commit 25952d4 into pingcap:master Feb 18, 2020
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bb7133 bb7133 bb7133 left review comments

@zimulala zimulala zimulala left review comments

@AilinKid AilinKid AilinKid left review comments

@tangenta tangenta tangenta approved these changes

@glorv glorv Awaiting requested review from glorv

@imtbkcat imtbkcat Awaiting requested review from imtbkcat

Assignees

No one assigned

Labels

contribution This PR is from a community contributor. sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM.

Projects

None yet

Milestone

v4.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

DROP TABLE IF EXISTS behavior difference

8 participants

@0xPoe @sre-bot @zimulala @tangenta @AilinKid @bb7133 @zz-jason @you06