-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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 default value of a newly added enum column #20798
Conversation
/reward 600 |
Reward success. |
odValue, err = zeroVal.ToString() | ||
if err != nil { | ||
return nil, errors.Trace(err) | ||
switch col.Tp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Set
also has this problem, could you fix it together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just compared with mysql, current logic's result is same as mysql.
below is mysql result:
mysql> create table t (id int primary key, c int);
Query OK, 0 rows affected (0.02 sec)
mysql> insert into t (id, c) values (1, 1), (2, 2);
Query OK, 2 rows affected (0.00 sec)
Records: 2 Duplicates: 0 Warnings: 0
mysql> alter table t add column cc set('a','b','c','d') not null;
Query OK, 0 rows affected (0.05 sec)
Records: 0 Duplicates: 0 Warnings: 0
mysql> update t set c = 2 where id = 1;
Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0
mysql> select * from t;
+----+------+----+
| id | c | cc |
+----+------+----+
| 1 | 2 | |
| 2 | 2 | |
+----+------+----+
2 rows in set (0.00 sec)
mysql> select * from t where cc = 0;
+----+------+----+
| id | c | cc |
+----+------+----+
| 1 | 2 | |
| 2 | 2 | |
+----+------+----+
2 rows in set (0.00 sec)
mysql> select * from t where cc = 1;
Empty set (0.00 sec)
mysql> insert into t (id, c) values (3, 3);
ERROR 1364 (HY000): Field 'cc' doesn't have a default value
Then TiDB result:
mysql> create table t (id int primary key, c int);
Query OK, 0 rows affected (0.00 sec)
mysql> insert into t (id, c) values (1, 1), (2, 2);
Query OK, 2 rows affected (0.00 sec)
Records: 2 Duplicates: 0 Warnings: 0
mysql> alter table t add column cc set('a','b','c','d') not null;
Query OK, 0 rows affected (0.01 sec)
mysql> update t set c = 2 where id = 1;
Query OK, 1 row affected (0.00 sec)
Rows matched: 1 Changed: 1 Warnings: 0
mysql> select * from t;
+----+------+----+
| id | c | cc |
+----+------+----+
| 1 | 2 | |
| 2 | 2 | |
+----+------+----+
2 rows in set (0.00 sec)
mysql> select * from t where cc = 0;
+----+------+----+
| id | c | cc |
+----+------+----+
| 1 | 2 | |
| 2 | 2 | |
+----+------+----+
2 rows in set (0.00 sec)
mysql> select * from t where cc = 1;
Empty set (0.00 sec)
mysql> insert into t (id, c) values (3, 3);
ERROR 1364 (HY000): Field 'cc' doesn't have a default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add these results as a test.
@wjhuang2016 after I add some debug log I found when create table the |
Yeah, this is an optimization. |
Do we need add |
No |
@wjhuang2016 PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
/run-all-tests |
@blacktear23 merge failed. |
/run-all-tests |
/run-all-tests |
/run-unit-test |
/merge |
/run-all-tests |
@blacktear23, Congratulations, you get 600 in this PR, and your total score is 600 in challenge program. |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-4.0 in PR #20998 |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-3.0 in PR #20999 |
What problem does this PR solve?
Issue Number: close #20741
Problem Summary:
The default value for new add enum column not perform correctly.
What is changed and how it works?
What's Changed:
Check column's type if is Enum just use first element for default value.
Related changes
Check List
Tests
Side effects
Release note