-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: year type should have an unsigned flag #7542
Changes from 6 commits
525b365
32000b8
900250b
0712e94
4583f65
67df709
50ace22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -394,6 +394,12 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o | |
col.Flag &= ^mysql.BinaryFlag | ||
col.Flag |= mysql.ZerofillFlag | ||
} | ||
// If you specify ZEROFILL for a numeric column, MySQL automatically adds the UNSIGNED attribute to the column. | ||
// See https://dev.mysql.com/doc/refman/5.7/en/numeric-type-overview.html for more details. | ||
// But some types like bit and year, won't show its unsigned flag in `show create table`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not support zerofill as #6688? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems it still could work in some client/driver, see #7525 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. myabe we need use |
||
if mysql.HasZerofillFlag(col.Flag) { | ||
col.Flag |= mysql.UnsignedFlag | ||
} | ||
err := checkPriKeyConstraint(col, hasDefaultValue, hasNullFlag, outPriKeyConstraint) | ||
if err != nil { | ||
return nil, nil, errors.Trace(err) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -362,13 +362,23 @@ func (s *testSuite) TestShow(c *C) { | |
|
||
// Test show create table year type | ||
tk.MustExec(`drop table if exists t`) | ||
tk.MustExec(`create table t(y year, x int, primary key(y));`) | ||
tk.MustExec(`create table t(y year unsigned signed zerofill zerofill, x int, primary key(y));`) | ||
tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", | ||
"t CREATE TABLE `t` (\n"+ | ||
" `y` year NOT NULL,\n"+ | ||
" `x` int(11) DEFAULT NULL,\n"+ | ||
" PRIMARY KEY (`y`)\n"+ | ||
") ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin")) | ||
|
||
// Test show create table with zerofill flag | ||
tk.MustExec(`drop table if exists t`) | ||
tk.MustExec(`create table t(id int primary key, val tinyint(10) zerofill);`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a year type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. year with zerofill? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not allowed, according to #7542 (comment) @shenli |
||
tk.MustQuery(`show create table t`).Check(testutil.RowsWithSep("|", | ||
"t CREATE TABLE `t` (\n"+ | ||
" `id` int(11) NOT NULL,\n"+ | ||
" `val` tinyint(10) UNSIGNED ZEROFILL DEFAULT NULL,\n"+ | ||
" PRIMARY KEY (`id`)\n"+ | ||
") ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin")) | ||
} | ||
|
||
func (s *testSuite) TestShowVisibility(c *C) { | ||
|
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.
Will this removal affect test-case's behavior?
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.
No.