Skip to content

Commit

Permalink
ddl: fix auto_increment is not compatible with MySQL (pingcap#14755)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rustin170506
Rustin170506 authored Feb 14, 2020
1 parent c59f339 commit 27f8c36
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 33 deletions.
2 changes: 2 additions & 0 deletions ddl/db_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,8 @@ func (s *testIntegrationSuite5) TestMySQLErrorCode(c *C) {
tk.MustGetErrCode(sql, mysql.ErrPrimaryCantHaveNull)
sql = "create table t2 (id int auto_increment);"
tk.MustGetErrCode(sql, mysql.ErrWrongAutoKey)
sql = "create table t2 (id int auto_increment, a int key);"
tk.MustGetErrCode(sql, mysql.ErrWrongAutoKey)
sql = "create table t2 (a datetime(2) default current_timestamp(3))"
tk.MustGetErrCode(sql, mysql.ErrInvalidDefault)
sql = "create table t2 (a datetime(2) default current_timestamp(2) on update current_timestamp)"
Expand Down
6 changes: 3 additions & 3 deletions ddl/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2212,7 +2212,7 @@ func (s *testDBSuite5) TestRepairTable(c *C) {
turnRepairModeAndInit(true)
defer turnRepairModeAndInit(false)
// Domain reload the tableInfo and add it into repairInfo.
s.tk.MustExec("CREATE TABLE origin (a int primary key, b varchar(10), c int auto_increment);")
s.tk.MustExec("CREATE TABLE origin (a int primary key auto_increment, b varchar(10), c int);")
// Repaired tableInfo has been filtered by `domain.InfoSchema()`, so get it in repairInfo.
originTableInfo, _ := domainutil.RepairInfo.GetRepairedTableInfoByTableName("test", "origin")

Expand Down Expand Up @@ -2243,7 +2243,7 @@ func (s *testDBSuite5) TestRepairTable(c *C) {
s.dom.DDL().(ddl.DDLForTest).SetHook(hook)

// Exec the repair statement to override the tableInfo.
s.tk.MustExec("admin repair table origin CREATE TABLE origin (a int primary key, b varchar(5), c int auto_increment);")
s.tk.MustExec("admin repair table origin CREATE TABLE origin (a int primary key auto_increment, b varchar(5), c int);")
c.Assert(repairErr, IsNil)

// Check the repaired tableInfo is exactly the same with old one in tableID, indexID, colID.
Expand All @@ -2265,7 +2265,7 @@ func (s *testDBSuite5) TestRepairTable(c *C) {

// Exec the show create table statement to make sure new tableInfo has been set.
result := s.tk.MustQuery("show create table origin")
c.Assert(result.Rows()[0][1], Equals, "CREATE TABLE `origin` (\n `a` int(11) NOT NULL,\n `b` varchar(5) DEFAULT NULL,\n `c` int(11) NOT NULL AUTO_INCREMENT,\n PRIMARY KEY (`a`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")
c.Assert(result.Rows()[0][1], Equals, "CREATE TABLE `origin` (\n `a` int(11) NOT NULL AUTO_INCREMENT,\n `b` varchar(5) DEFAULT NULL,\n `c` int(11) DEFAULT NULL,\n PRIMARY KEY (`a`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")

}

Expand Down
63 changes: 33 additions & 30 deletions planner/core/preprocess.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,15 +240,15 @@ func (p *preprocessor) Leave(in ast.Node) (out ast.Node, ok bool) {
return in, p.err == nil
}

func checkAutoIncrementOp(colDef *ast.ColumnDef, num int) (bool, error) {
func checkAutoIncrementOp(colDef *ast.ColumnDef, index int) (bool, error) {
var hasAutoIncrement bool

if colDef.Options[num].Tp == ast.ColumnOptionAutoIncrement {
if colDef.Options[index].Tp == ast.ColumnOptionAutoIncrement {
hasAutoIncrement = true
if len(colDef.Options) == num+1 {
if len(colDef.Options) == index+1 {
return hasAutoIncrement, nil
}
for _, op := range colDef.Options[num+1:] {
for _, op := range colDef.Options[index+1:] {
if op.Tp == ast.ColumnOptionDefaultValue {
if tmp, ok := op.Expr.(*driver.ValueExpr); ok {
if !tmp.Datum.IsNull() {
Expand All @@ -258,13 +258,13 @@ func checkAutoIncrementOp(colDef *ast.ColumnDef, num int) (bool, error) {
}
}
}
if colDef.Options[num].Tp == ast.ColumnOptionDefaultValue && len(colDef.Options) != num+1 {
if tmp, ok := colDef.Options[num].Expr.(*driver.ValueExpr); ok {
if colDef.Options[index].Tp == ast.ColumnOptionDefaultValue && len(colDef.Options) != index+1 {
if tmp, ok := colDef.Options[index].Expr.(*driver.ValueExpr); ok {
if tmp.Datum.IsNull() {
return hasAutoIncrement, nil
}
}
for _, op := range colDef.Options[num+1:] {
for _, op := range colDef.Options[index+1:] {
if op.Tp == ast.ColumnOptionAutoIncrement {
return hasAutoIncrement, errors.Errorf("Invalid default value for '%s'", colDef.Name.Name.O)
}
Expand Down Expand Up @@ -292,14 +292,11 @@ func isConstraintKeyTp(constraints []*ast.Constraint, colDef *ast.ColumnDef) boo
}

func (p *preprocessor) checkAutoIncrement(stmt *ast.CreateTableStmt) {
var (
isKey bool
count int
autoIncrementCol *ast.ColumnDef
)
autoIncrementCols := make(map[*ast.ColumnDef]bool)

for _, colDef := range stmt.Cols {
var hasAutoIncrement bool
var isKey bool
for i, op := range colDef.Options {
ok, err := checkAutoIncrementOp(colDef, i)
if err != nil {
Expand All @@ -315,33 +312,39 @@ func (p *preprocessor) checkAutoIncrement(stmt *ast.CreateTableStmt) {
}
}
if hasAutoIncrement {
count++
autoIncrementCol = colDef
autoIncrementCols[colDef] = isKey
}
}

if count < 1 {
if len(autoIncrementCols) < 1 {
return
}
if !isKey {
isKey = isConstraintKeyTp(stmt.Constraints, autoIncrementCol)
if len(autoIncrementCols) > 1 {
p.err = autoid.ErrWrongAutoKey.GenWithStackByArgs()
return
}
autoIncrementMustBeKey := true
for _, opt := range stmt.Options {
if opt.Tp == ast.TableOptionEngine && strings.EqualFold(opt.StrValue, "MyISAM") {
autoIncrementMustBeKey = false
// Only have one auto_increment col.
for col, isKey := range autoIncrementCols {
if !isKey {
isKey = isConstraintKeyTp(stmt.Constraints, col)
}
autoIncrementMustBeKey := true
for _, opt := range stmt.Options {
if opt.Tp == ast.TableOptionEngine && strings.EqualFold(opt.StrValue, "MyISAM") {
autoIncrementMustBeKey = false
}
}
if autoIncrementMustBeKey && !isKey {
p.err = autoid.ErrWrongAutoKey.GenWithStackByArgs()
}
switch col.Tp.Tp {
case mysql.TypeTiny, mysql.TypeShort, mysql.TypeLong,
mysql.TypeFloat, mysql.TypeDouble, mysql.TypeLonglong, mysql.TypeInt24:
default:
p.err = errors.Errorf("Incorrect column specifier for column '%s'", col.Name.Name.O)
}
}
if (autoIncrementMustBeKey && !isKey) || count > 1 {
p.err = autoid.ErrWrongAutoKey.GenWithStackByArgs()
}

switch autoIncrementCol.Tp.Tp {
case mysql.TypeTiny, mysql.TypeShort, mysql.TypeLong,
mysql.TypeFloat, mysql.TypeDouble, mysql.TypeLonglong, mysql.TypeInt24:
default:
p.err = errors.Errorf("Incorrect column specifier for column '%s'", autoIncrementCol.Name.Name.O)
}
}

// checkUnionSelectList checks union's selectList.
Expand Down
9 changes: 9 additions & 0 deletions planner/core/preprocess_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/ddl"
"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/meta/autoid"
"github.com/pingcap/tidb/planner/core"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/sessionctx"
Expand Down Expand Up @@ -206,6 +207,14 @@ func (s *testValidatorSuite) TestValidator(c *C) {
{"CREATE TABLE t1 (id INT NOT NULL, c1 VARCHAR(20) AS ('foo') VIRTUAL KEY NULL, PRIMARY KEY (id));", false, core.ErrUnsupportedOnGeneratedColumn},
{"CREATE TABLE t1 (id INT NOT NULL, c1 VARCHAR(20) AS ('foo') VIRTUAL KEY NOT NULL, PRIMARY KEY (id));", false, core.ErrUnsupportedOnGeneratedColumn},
{"create table t (a DOUBLE NULL, b_sto DOUBLE GENERATED ALWAYS AS (a + 2) STORED UNIQUE KEY NOT NULL PRIMARY KEY);", false, nil},

// issue 13032
{"CREATE TABLE origin (a int primary key, b varchar(10), c int auto_increment);", false, autoid.ErrWrongAutoKey},
{"CREATE TABLE origin (a int auto_increment, b int key);", false, autoid.ErrWrongAutoKey},
{"CREATE TABLE origin (a int auto_increment, b int unique);", false, autoid.ErrWrongAutoKey},
{"CREATE TABLE origin (a int primary key auto_increment, b int);", false, nil},
{"CREATE TABLE origin (a int unique auto_increment, b int);", false, nil},
{"CREATE TABLE origin (a int key auto_increment, b int);", false, nil},
}

store, dom, err := newStoreWithBootstrap()
Expand Down

0 comments on commit 27f8c36

Please sign in to comment.