Skip to content

ddl: disallow dropping index on auto_increment column (#11360)#11399

Merged
zz-jason merged 4 commits into
pingcap:release-3.0from
tangenta:release-3.0-drop-autoinc-col
Jul 26, 2019
Merged

ddl: disallow dropping index on auto_increment column (#11360)#11399
zz-jason merged 4 commits into
pingcap:release-3.0from
tangenta:release-3.0-drop-autoinc-col

Conversation

@tangenta
Copy link
Copy Markdown
Contributor

@tangenta tangenta commented Jul 24, 2019

Cherry-pick #11360.

winkyao
winkyao reviewed Jul 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@winkyao winkyao 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 tangenta added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 24, 2019
zz-jason
zz-jason reviewed Jul 24, 2019
View reviewed changes
Comment thread planner/core/preprocess.go
}
if (autoIncrementMustBeKey && !isKey) || count > 1 {
p.err = errors.New("Incorrect table definition; there can be only one auto column and it must be defined as a key")
p.err = autoid.ErrWrongAutoKey.GenWithStackByArgs()
Copy link
Copy Markdown
Member

@zz-jason zz-jason Jul 24, 2019

Choose a reason for hiding this comment

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

If arguments is empty, I think we can use GenWithStack()

Copy link
Copy Markdown
Contributor Author

@tangenta tangenta Jul 24, 2019

Choose a reason for hiding this comment

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

@zz-jason GenWithStack() accepts a formatted string, but there is no such string here.

zz-jason
zz-jason approved these changes Jul 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 24, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 24, 2019

/run-all-tests

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 24, 2019

@tangenta merge failed.

@tangenta
Copy link
Copy Markdown
Contributor Author

tangenta commented Jul 24, 2019 

[2019-07-24T11:07:45.144Z] ----------------------------------------------------------------------
[2019-07-24T11:07:45.144Z] FAIL: client_fail_test.go:32: testClientSuite.TestPanicInRecvLoop
[2019-07-24T11:07:45.144Z] 
[2019-07-24T11:07:45.144Z] client_fail_test.go:57:
[2019-07-24T11:07:45.144Z]     c.Assert(err, IsNil)
[2019-07-24T11:07:45.144Z] ... value *errors.fundamental = injected error in batchRecvLoop ("injected error in batchRecvLoop")

/run-check_dev_2

@tangenta
Copy link
Copy Markdown
Contributor Author

tangenta commented Jul 24, 2019

/rebuild

@tangenta
Copy link
Copy Markdown
Contributor Author

tangenta commented Jul 24, 2019 •
edited
Loading 

----------------------------------------------------------------------
FAIL: client_fail_test.go:32: testClientSuite.TestPanicInRecvLoop

client_fail_test.go:57:
    c.Assert(err, IsNil)
... value *errors.fundamental = injected error in batchRecvLoop ("injected error in batchRecvLoop")

This error has occurred for the second time. @crazycs520 PTAL?

@zz-jason zz-jason merged commit 1124627 into pingcap:release-3.0 Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zz-jason zz-jason zz-jason approved these changes

+1 more reviewer

@winkyao winkyao winkyao left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status/can-merge Indicates a PR has been approved by a committer.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tangenta @sre-bot @zz-jason @winkyao