Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ddl: fix create and alter check constraints problems #47633

Merged
merged 5 commits into from
Nov 8, 2023
Merged

ddl: fix create and alter check constraints problems #47633

merged 5 commits into from
Nov 8, 2023

Conversation

jiyfhust
Copy link
Contributor

@jiyfhust jiyfhust commented Oct 14, 2023 •
edited
Loading

What problem does this PR solve?

Issue Number: close #47567 close #47631 close #47632

It seems also close #44737

Problem Summary:

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-tests-checked do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. and removed do-not-merge/needs-tests-checked labels Oct 14, 2023
@ti-chi-bot
Copy link

ti-chi-bot bot commented Oct 14, 2023

Hi @jiyfhust. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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/test-infra repository.

@tiprow
Copy link

tiprow bot commented Oct 14, 2023

Hi @jiyfhust. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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/test-infra repository.

@jiyfhust
Copy link
Contributor Author

/cc @fzzf678 @zimulala @CbcWestwolf

@jiyfhust
Copy link
Contributor Author

/ok-to-test

@ti-chi-bot ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Oct 27, 2023
@codecov
Copy link

codecov bot commented Oct 27, 2023 •
edited
Loading

Codecov Report

Merging #47633 (a9b1fd8) into master (30288c7) will increase coverage by 1.1152%.
Report is 102 commits behind head on master.
The diff coverage is 74.1935%.

Additional details and impacted files 
@@               Coverage Diff                @@
##             master     #47633        +/-   ##
================================================
+ Coverage   71.6145%   72.7298%   +1.1152%     
================================================
  Files          1401       1430        +29     
  Lines        405923     429238     +23315     
================================================
+ Hits         290700     312184     +21484     
- Misses        95448      98021      +2573     
+ Partials      19775      19033       -742
Flag Coverage Δ
integration 45.0973% <72.5806%> (?)
unit 71.6251% <41.9354%> (+0.0105%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 54.0503% <ø> (ø)
parser ∅ <ø> (∅)
br 47.1608% <ø> (-5.7600%) ⬇️

CbcWestwolf
CbcWestwolf reviewed Oct 30, 2023
View reviewed changes
pkg/ddl/constraint_test.go Outdated
tk.MustExec("use test")
tk.MustExec("CREATE TABLE `t` (`a` int(11) DEFAULT NULL)")
tk.MustExec("insert t values(1)")
tk.MustGetErrMsg("alter table t ADD CONSTRAINT chk CHECK (a > 1) ENFORCED", "[ddl:3819]Check constraint 'chk' is violated.")
Copy link
Member

@CbcWestwolf CbcWestwolf Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why are there two same SQLs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As issue #47632 reproduce steps，the first sql will rollback failed, and the second will report "Duplicate check constraint name" error.

Copy link
Member

Choose a reason for hiding this comment

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

Got it!

@CbcWestwolf CbcWestwolf changed the title ddl: fix create、alter check constraints problems ddl: fix create and alter check constraints problems Oct 30, 2023
@jiyfhust
Copy link
Contributor Author

/retest

@ti-chi-bot ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 1, 2023
@ti-chi-bot ti-chi-bot bot requested a review from tangenta November 6, 2023 07:35
@jiyfhust
Copy link
Contributor Author

jiyfhust commented Nov 7, 2023

/cc Benjamin2037 tangenta wjhuang2016 ywqzzy zimulala

@ti-chi-bot ti-chi-bot
Copy link

ti-chi-bot bot commented Nov 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CbcWestwolf, tangenta, tiancaiamao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the approved label Nov 8, 2023
@jiyfhust
Copy link
Contributor Author

jiyfhust commented Nov 8, 2023

/retest

@ti-chi-bot ti-chi-bot bot merged commit 14ff60d into pingcap:master Nov 8, 2023
@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Closed
13 tasks
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-5.4: #48386.

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 8, 2023
@jiyfhust @ti-chi-bot
This is an automated cherry-pick of pingcap#47633
21a37de 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-6.1: #48387.

@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Closed
13 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 8, 2023
@jiyfhust @ti-chi-bot
This is an automated cherry-pick of pingcap#47633
f8f4b44 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-6.5: #48388.

@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Closed
13 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 8, 2023
@jiyfhust @ti-chi-bot
This is an automated cherry-pick of pingcap#47633
f32bdb5 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.1: #48389.

@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Closed
13 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 8, 2023
@jiyfhust @ti-chi-bot
This is an automated cherry-pick of pingcap#47633
39cf120 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.5: #48390.

@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Merged
13 tasks
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-5.3: #48391.

@ti-chi-bot ti-chi-bot mentioned this pull request Nov 8, 2023
Closed
13 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Nov 8, 2023
@jiyfhust @ti-chi-bot
This is an automated cherry-pick of pingcap#47633
79a9989 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@jiyfhust jiyfhust deleted the fix_check_ddl branch November 8, 2023 03:01
ti-chi-bot bot pushed a commit that referenced this pull request Feb 1, 2024
@ti-chi-bot
ddl: fix create and alter check constraints problems (#47633) (#48390)
181f71c 
close #47567, close #47631, close #47632
@ti-chi-bot ti-chi-bot removed the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tangenta tangenta tangenta approved these changes

@CbcWestwolf CbcWestwolf CbcWestwolf approved these changes

@tiancaiamao tiancaiamao tiancaiamao approved these changes

@fzzf678 fzzf678 Awaiting requested review from fzzf678

@zimulala zimulala Awaiting requested review from zimulala

@Benjamin2037 Benjamin2037 Awaiting requested review from Benjamin2037

@wjhuang2016 wjhuang2016 Awaiting requested review from wjhuang2016

@ywqzzy ywqzzy Awaiting requested review from ywqzzy

Assignees
No one assigned
Labels
approved lgtm needs-cherry-pick-release-5.3 Type: Need cherry pick to release-5.3 needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
5 participants
@jiyfhust @tiancaiamao @CbcWestwolf @ti-chi-bot @tangenta