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 the set's default value where create table #12267

Merged
merged 6 commits into from
Sep 26, 2019
Merged

ddl: fix the set's default value where create table #12267

merged 6 commits into from
Sep 26, 2019

Conversation

zimulala
Copy link
Contributor

@zimulala zimulala commented Sep 18, 2019 •
edited
Loading

What problem does this PR solve?

What is changed and how it works?

  • Call ParseSetValue when the set type default value is int type. Mysql implementation also has problems, as follows:
mysql> create table t_set (a int, b set('1', '5') default 1);
Query OK, 0 rows affected (0.02 sec)
mysql> create table t (a int, b set('1', '5') default 2);
ERROR 1067 (42000): Invalid default value for 'b'
mysql> create table t (a int, b set('4', '5') default 1);

So code implementation reference MySQL set type.

  • Check if the default value exists in the set.

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Write release note for bug-fix or new feature.

zhexuany
zhexuany reviewed Sep 23, 2019
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@zimulala
Copy link
Contributor Author

PTAL @zhexuany

zhexuany
zhexuany reviewed Sep 24, 2019
View reviewed changes
Copy link
Contributor

@zhexuany zhexuany left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Sep 24, 2019 •
edited
Loading

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12267   +/-   ##
===========================================
  Coverage   79.9721%   79.9721%           
===========================================
  Files           461        461           
  Lines        102762     102762           
===========================================
  Hits          82181      82181           
  Misses        14773      14773           
  Partials       5808       5808

@zimulala
Copy link
Contributor Author

PTAL @crazycs520 @bb7133

lonng
lonng reviewed Sep 26, 2019
View reviewed changes
ddl/ddl_api.go Outdated Show resolved Hide resolved
lonng
lonng reviewed Sep 26, 2019
View reviewed changes
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

Reset LGTM

@bb7133
Copy link
Member

bb7133 commented Sep 26, 2019

LGTM, please address @lonng 's comment

crazycs520
crazycs520 reviewed Sep 26, 2019
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 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 status/LGT3 The PR has already had 3 LGTM. status/can-merge Indicates a PR has been approved by a committer. labels Sep 26, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

@zimulala merge failed.

@zimulala
Copy link
Contributor Author

/run-integration-compatibility-test

@zimulala zimulala merged commit 20bdf44 into pingcap:master Sep 26, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

cherry pick to release-2.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

cherry pick to release-3.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019

cherry pick to release-3.1 failed

@zimulala zimulala deleted the set-default-val branch September 26, 2019 04:22
zimulala added a commit to zimulala/tidb that referenced this pull request Sep 27, 2019
@zimulala
ddl: fix the set's default value where create table (pingcap#12267)
78d3db9
zimulala added a commit to zimulala/tidb that referenced this pull request Sep 27, 2019
@zimulala
ddl: fix the set's default value where create table (pingcap#12267)
0b86d9a
This was referenced Sep 27, 2019
Merged
Merged
zimulala added a commit to zimulala/tidb that referenced this pull request Sep 29, 2019
@zimulala
ddl: fix the set's default value where create table (pingcap#12267)
586eacd
@zimulala zimulala mentioned this pull request Sep 29, 2019
Closed
zimulala added a commit that referenced this pull request Sep 30, 2019
@zimulala
ddl: fix the set's default value where create table (#12267) (#12452)
a046953
zimulala added a commit that referenced this pull request Sep 30, 2019
@zimulala
ddl: fix the set's default value where create table (#12267) (#12451)
1877cb7
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
@xiongjiwei xiongjiwei mentioned this pull request Nov 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lonng lonng lonng left review comments

@zhexuany zhexuany zhexuany left review comments

@crazycs520 crazycs520 crazycs520 approved these changes

Assignees
No one assigned
Labels
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. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SET type has inconsistent behavior compared with MySQL
7 participants
@zimulala @bb7133 @sre-bot @lonng @zhexuany @crazycs520 @you06