-
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: refine error messages in unsupported column options #11065
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11065 +/- ##
================================================
- Coverage 81.5925% 81.2529% -0.3396%
================================================
Files 424 423 -1
Lines 91626 90025 -1601
================================================
- Hits 74760 73148 -1612
- Misses 11540 11578 +38
+ Partials 5326 5299 -27 |
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
35a2c70
to
34720ac
Compare
I have tried following SQL on the code, but got following error:
|
@AilinKid Which TiDB version did you use? mysql root@127.0.0.1:test> CREATE TABLE t6(a char(1))DEFAULT CHARSET=utf8mb4 COL
LATE=utf8mb4_general_ci ;
Query OK, 0 rows affected
Time: 0.014s
mysql root@127.0.0.1:test> ALTER TABLE t6 MODIFY COLUMN a char(1) CHARACTER SET
utf8mb4 COLLATE utf8mb4_general_ci ;
Query OK, 0 rows affected
Time: 0.014s
mysql root@127.0.0.1:test> select tidb_version();
1 row in set
Time: 0.029s
mysql root@127.0.0.1:test> select tidb_version();
+--------------------------------------------------------------------------+
| tidb_version() |
+--------------------------------------------------------------------------+
| Release Version: v3.0.0-rc.1-309-g34720acda |
| Git Commit Hash: 34720acda1eec86ff93494c8890cb2a1b4d6d478 |
| Git Branch: changing-collation |
| UTC Build Time: 2019-07-10 09:12:23 |
| GoVersion: go version go1.12.5 linux/amd64 |
| Race Enabled: false |
| TiKV Min Version: 2.1.0-alpha.1-ff3dd160846b7d1aed9079c389fc188f7f5ea13e |
| Check Table Before Drop: false |
+--------------------------------------------------------------------------+
1 row in set
Time: 0.015s git log: commit 34720acda1eec86ff93494c8890cb2a1b4d6d478 (HEAD -> changing-collation, origin/changing-collation)
Author: tangenta <[email protected]>
Date: Wed Jul 10 15:39:13 2019 +0800
refine error message |
ec4d9fa
to
32e825f
Compare
32e825f
to
96b24b7
Compare
/run-all-tests |
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
It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @tangenta PTAL. |
What problem does this PR solve?
Fix #10731 and fix #11052.
What is changed and how it works?
Add more cases in
switch
inprocessColumnOptions()
, avoiding the obscure error message in unsupported modify column %!s(ast.ColumnOptionType=12) #10731.getModifiableColumnJob()
is used to build aActionModifyColumn
DDL job. It contains two methods:modifiable()
, check whether the FieldType of the origin column can be changed into the new column. The checks include charset, collation, types, Flen/Decimal, etc.processColumnOptions()
, set FieldType of the new column, according toOption
inColumnDef
, which is filled by SQL parser.Obviously, we should first set the FieldType and then check it. So
processColumnOptions()
should come first.Check List
Tests
Related changes