ddl: fix a bug that create two views with same name success #58770
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @tiancaiamao. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. 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 kubernetes-sigs/prow repository. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #58770 +/- ##
================================================
+ Coverage 73.0885% 75.2524% +2.1639%
================================================
Files 1676 1722 +46
Lines 463646 478936 +15290
================================================
+ Hits 338872 360411 +21539
+ Misses 103927 95652 -8275
- Partials 20847 22873 +2026
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/retest |
|
@tiancaiamao: Cannot trigger testing until a trusted user reviews the PR and leaves an 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 kubernetes-sigs/prow repository. |
tiancaiamao
requested review from
D3Hunter and
YangKeao
and removed request for
D3Hunter
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: YangKeao The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
| @@ -330,6 +330,11 @@ func onCreateView(jobCtx *jobContext, job *model.Job) (ver int64, _ error) { | |||
| return ver, errors.Trace(err) | |||
| } | |||
| } | |||
| if oldTableID > 0 && !orReplace { | |||
There was a problem hiding this comment.
the branch at L325 is the handling of this case, but set at the wrong location https://github.com/pingcap/tidb/pull/10170/files#diff-9dd5a0976146b94bb3074ca04e8131dffc03096505755440e64760b42bbde0fdR106-R110
There was a problem hiding this comment.
err is nil, the code does not go to the branch. @D3Hunter
There was a problem hiding this comment.
yes, i mean you can set the err to TableAlreadyExist if tableID>0, then that branch will work. currently that branch is a dead one
There was a problem hiding this comment.
You mean this?
if oldTableID > 0 && err == nil {
err = infoschema.ErrTableExists
}
What's the benefit over current code? @D3Hunter
There was a problem hiding this comment.
L325 is a dead branch now, I hope in your PR you can fix it, or make use of it
What problem does this PR solve?
Issue Number: close #58769
Problem Summary:
What changed and how does it work?
After one 'create view' ddl job delivered (queuing), the another
create viewddl job come.The check for table name duplicated is incorrect.
When old table exists and there is no
or replace, it should report error.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.