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.

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

ddl: fix sequence default value will cause primary-key occur not-null error #16437

Merged
merged 8 commits into from
Apr 17, 2020

Conversation

AilinKid
Copy link
Contributor

@AilinKid AilinKid commented Apr 15, 2020

What problem does this PR solve?

close bug competition issue #11

What is changed and how it works?

if the default is expr(sequence expr), return nil directly in function checkDefaultValue
otherwise will cause some primary key check problem.

Check List

Tests

  • Unit test
  • Integration test

Release note: ddl: fix sequence default value will cause primary-key occur not-null error

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@AilinKid AilinKid added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 15, 2020
@codecov
Copy link

codecov bot commented Apr 16, 2020

Codecov Report

Merging #16437 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #16437   +/-   ##
===========================================
  Coverage   80.6005%   80.6005%           
===========================================
  Files           506        506           
  Lines        137633     137633           
===========================================
  Hits         110933     110933           
  Misses        18119      18119           
  Partials       8581       8581           

@AilinKid AilinKid added the type/bugfix This PR fixes a bug. label Apr 16, 2020
@AilinKid
Copy link
Contributor Author

/run-unit-test

Copy link
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

@zimulala zimulala added the status/can-merge Indicates a PR has been approved by a committer. label Apr 16, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 16, 2020

/run-all-tests

@AilinKid AilinKid added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 16, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 16, 2020

@AilinKid merge failed.

@AilinKid
Copy link
Contributor Author

/run-integration-copr-test

@AilinKid
Copy link
Contributor Author

/run-all-tests

@AilinKid
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Apr 17, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Apr 17, 2020

@AilinKid merge failed.

@AilinKid
Copy link
Contributor Author

/run-unit-test

@AilinKid
Copy link
Contributor Author

/run-unit-test

@sre-bot
Copy link
Contributor

sre-bot commented Apr 17, 2020

cherry pick to release-4.0 in PR #16510

@AilinKid
Copy link
Contributor Author

/build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants