Skip to content

Commit

Permalink
ddl : fix column collate should use table's if it has when create tab…
Browse files Browse the repository at this point in the history 
…le or alter table. (#13119)

fix column's collation should use table's collation
  • Loading branch information
@AilinKid
AilinKid authored Nov 6, 2019
1 parent a28fc71 commit 8d545a1
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 39 deletions.
2 changes: 1 addition & 1 deletion ddl/column_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ func (s *testColumnSuite) colDefStrToFieldType(c *C, str string) *types.FieldTyp
stmt, err := parser.New().ParseOneStmt(sqlA, "", "")
c.Assert(err, IsNil)
colDef := stmt.(*ast.AlterTableStmt).Specs[0].NewColumns[0]
col, _, err := buildColumnAndConstraint(nil, 0, colDef, nil, mysql.DefaultCharset, mysql.DefaultCharset)
col, _, err := buildColumnAndConstraint(nil, 0, colDef, nil, mysql.DefaultCharset, "", mysql.DefaultCharset, "")
c.Assert(err, IsNil)
return &col.FieldType
}
Expand Down
13 changes: 12 additions & 1 deletion ddl/db_change_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ func (s *testStateChangeSuite) TestShowCreateTable(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("create table t (id int)")
tk.MustExec("create table t2 (a int, b varchar(10)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci")

var checkErr error
testCases := []struct {
Expand All @@ -94,6 +95,10 @@ func (s *testStateChangeSuite) TestShowCreateTable(c *C) {
"CREATE TABLE `t` (\n `id` int(11) DEFAULT NULL,\n KEY `idx` (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"},
{"alter table t add column c int",
"CREATE TABLE `t` (\n `id` int(11) DEFAULT NULL,\n KEY `idx` (`id`),\n KEY `idx1` (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"},
{"alter table t2 add column c varchar(1)",
"CREATE TABLE `t2` (\n `a` int(11) DEFAULT NULL,\n `b` varchar(10) COLLATE utf8mb4_general_ci DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci"},
{"alter table t2 add column d varchar(1)",
"CREATE TABLE `t2` (\n `a` int(11) DEFAULT NULL,\n `b` varchar(10) COLLATE utf8mb4_general_ci DEFAULT NULL,\n `c` varchar(1) COLLATE utf8mb4_general_ci DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci"},
}
prevState := model.StateNone
callback := &ddl.TestDDLCallback{}
Expand All @@ -106,7 +111,13 @@ func (s *testStateChangeSuite) TestShowCreateTable(c *C) {
currTestCaseOffset++
}
if job.SchemaState != model.StatePublic {
result := tk.MustQuery("show create table t")
var result *testkit.Result
tbl2 := testGetTableByName(c, tk.Se, "test", "t2")
if job.TableID == tbl2.Meta().ID {
result = tk.MustQuery("show create table t2")
} else {
result = tk.MustQuery("show create table t")
}
got := result.Rows()[0][1]
expected := testCases[currTestCaseOffset].expectedRet
if got != expected {
Expand Down
14 changes: 14 additions & 0 deletions ddl/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,21 @@ func (s *testDBSuite1) TestCreateTable(c *C) {
_, err = s.tk.Exec("CREATE TABLE `t` (`a` int) DEFAULT CHARSET=abcdefg")
c.Assert(err, NotNil)

_, err = s.tk.Exec("CREATE TABLE `collateTest` (`a` int, `b` varchar(10)) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_slovak_ci")
c.Assert(err, IsNil)
result := s.tk.MustQuery("show create table collateTest")
got := result.Rows()[0][1]
c.Assert(got, Equals, "CREATE TABLE `collateTest` (\n `a` int(11) DEFAULT NULL,\n `b` varchar(10) COLLATE utf8_slovak_ci DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_slovak_ci")

s.tk.MustExec("create database test2 default charset utf8 collate utf8_general_ci")
s.tk.MustExec("use test2")
s.tk.MustExec("create table dbCollateTest (a varchar(10))")
result = s.tk.MustQuery("show create table dbCollateTest")
got = result.Rows()[0][1]
c.Assert(got, Equals, "CREATE TABLE `dbCollateTest` (\n `a` varchar(10) COLLATE utf8_general_ci DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_general_ci")

// test for enum column
s.tk.MustExec("use test")
failSQL := "create table t_enum (a enum('e','e'));"
s.tk.MustGetErrCode(failSQL, tmysql.ErrDuplicatedValueInType)
failSQL = "create table t_enum (a enum('e','E'));"
Expand Down
89 changes: 55 additions & 34 deletions ddl/ddl_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func setColumnFlagWithConstraint(colMap map[string]*table.Column, v *ast.Constra
}

func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef,
constraints []*ast.Constraint, tblCharset, dbCharset string) ([]*table.Column, []*ast.Constraint, error) {
constraints []*ast.Constraint, tblCharset, tblCollate, dbCharset, dbCollate string) ([]*table.Column, []*ast.Constraint, error) {
colMap := map[string]*table.Column{}
// outPriKeyConstraint is the primary key constraint out of column definition. such as: create table t1 (id int , age int, primary key(id));
var outPriKeyConstraint *ast.Constraint
Expand All @@ -266,7 +266,7 @@ func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef
}
cols := make([]*table.Column, 0, len(colDefs))
for i, colDef := range colDefs {
col, cts, err := buildColumnAndConstraint(ctx, i, colDef, outPriKeyConstraint, tblCharset, dbCharset)
col, cts, err := buildColumnAndConstraint(ctx, i, colDef, outPriKeyConstraint, tblCharset, tblCollate, dbCharset, dbCollate)
if err != nil {
return nil, nil, errors.Trace(err)
}
Expand All @@ -282,23 +282,32 @@ func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef
return cols, constraints, nil
}

// ResolveCharsetCollation will resolve the charset by the order: table charset > database charset > server default charset.
func ResolveCharsetCollation(tblCharset, dbCharset string) (string, string, error) {
// ResolveCharsetCollation will resolve the charset by the order: table charset > database charset > server default charset,
// and it will also resolve the collate by the order: table collate > database collate > server default collate.
func ResolveCharsetCollation(tblCharset, tblCollate, dbCharset, dbCollate string) (string, string, error) {
if len(tblCharset) != 0 {
defCollate, err := charset.GetDefaultCollation(tblCharset)
if err != nil {
// return terror is better.
return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(tblCharset)
// tblCollate is not specified by user.
if len(tblCollate) == 0 {
defCollate, err := charset.GetDefaultCollation(tblCharset)
if err != nil {
// return terror is better.
return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(tblCharset)
}
return tblCharset, defCollate, nil
}
return tblCharset, defCollate, nil
return tblCharset, tblCollate, nil
}

if len(dbCharset) != 0 {
defCollate, err := charset.GetDefaultCollation(dbCharset)
if err != nil {
return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(dbCharset)
// dbCollate is not specified by user.
if len(dbCollate) == 0 {
defCollate, err := charset.GetDefaultCollation(dbCharset)
if err != nil {
return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(dbCharset)
}
return dbCharset, defCollate, nil
}
return dbCharset, defCollate, errors.Trace(err)
return dbCharset, dbCollate, nil
}

charset, collate := charset.GetDefaultCharsetAndCollate()
Expand All @@ -315,15 +324,15 @@ func typesNeedCharset(tp byte) bool {
return false
}

func setCharsetCollationFlenDecimal(tp *types.FieldType, specifiedCollates []string, tblCharset string, dbCharset string) error {
func setCharsetCollationFlenDecimal(tp *types.FieldType, specifiedCollates []string, tblCharset, tblCollate, dbCharset, dbCollate string) error {
tp.Charset = strings.ToLower(tp.Charset)
tp.Collate = strings.ToLower(tp.Collate)
if len(tp.Charset) == 0 {
if typesNeedCharset(tp.Tp) {
if len(specifiedCollates) == 0 {
// Both the charset and collate are not specified.
var err error
tp.Charset, tp.Collate, err = ResolveCharsetCollation(tblCharset, dbCharset)
tp.Charset, tp.Collate, err = ResolveCharsetCollation(tblCharset, tblCollate, dbCharset, dbCollate)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -394,11 +403,11 @@ func setCharsetCollationFlenDecimal(tp *types.FieldType, specifiedCollates []str

// outPriKeyConstraint is the primary key constraint out of column definition. such as: create table t1 (id int , age int, primary key(id));
func buildColumnAndConstraint(ctx sessionctx.Context, offset int,
colDef *ast.ColumnDef, outPriKeyConstraint *ast.Constraint, tblCharset, dbCharset string) (*table.Column, []*ast.Constraint, error) {
colDef *ast.ColumnDef, outPriKeyConstraint *ast.Constraint, tblCharset, tblCollate, dbCharset, dbCollate string) (*table.Column, []*ast.Constraint, error) {
// specifiedCollates refers to collates in colDef.Options, should handle them together.
specifiedCollates := extractCollateFromOption(colDef)

if err := setCharsetCollationFlenDecimal(colDef.Tp, specifiedCollates, tblCharset, dbCharset); err != nil {
if err := setCharsetCollationFlenDecimal(colDef.Tp, specifiedCollates, tblCharset, tblCollate, dbCharset, dbCollate); err != nil {
return nil, nil, errors.Trace(err)
}
col, cts, err := columnDefToCol(ctx, offset, colDef, outPriKeyConstraint)
Expand Down Expand Up @@ -1279,10 +1288,10 @@ func buildTableInfoWithLike(ident ast.Ident, referTblInfo *model.TableInfo) mode
// The SQL string should be a create table statement.
// Don't use this function to build a partitioned table.
func BuildTableInfoFromAST(s *ast.CreateTableStmt) (*model.TableInfo, error) {
return buildTableInfoWithCheck(mock.NewContext(), nil, s, mysql.DefaultCharset)
return buildTableInfoWithCheck(mock.NewContext(), nil, s, mysql.DefaultCharset, "")
}

func buildTableInfoWithCheck(ctx sessionctx.Context, d *ddl, s *ast.CreateTableStmt, dbCharset string) (*model.TableInfo, error) {
func buildTableInfoWithCheck(ctx sessionctx.Context, d *ddl, s *ast.CreateTableStmt, dbCharset, dbCollate string) (*model.TableInfo, error) {
ident := ast.Ident{Schema: s.Table.Schema, Name: s.Table.Name}
colDefs := s.Cols
colObjects := make([]interface{}, 0, len(colDefs))
Expand All @@ -1309,9 +1318,9 @@ func buildTableInfoWithCheck(ctx sessionctx.Context, d *ddl, s *ast.CreateTableS
return nil, errors.Trace(err)
}

tableCharset := findTableOptionCharset(s.Options)
tableCharset, tableCollate := findTableOptionCharsetAndCollate(s.Options)
// The column charset haven't been resolved here.
cols, newConstraints, err := buildColumnsAndConstraints(ctx, colDefs, s.Constraints, tableCharset, dbCharset)
cols, newConstraints, err := buildColumnsAndConstraints(ctx, colDefs, s.Constraints, tableCharset, tableCollate, dbCharset, dbCollate)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -1353,7 +1362,7 @@ func buildTableInfoWithCheck(ctx sessionctx.Context, d *ddl, s *ast.CreateTableS
return nil, errors.Trace(err)
}

if err = resolveDefaultTableCharsetAndCollation(tbInfo, dbCharset); err != nil {
if err = resolveDefaultTableCharsetAndCollation(tbInfo, dbCharset, dbCollate); err != nil {
return nil, errors.Trace(err)
}

Expand Down Expand Up @@ -1384,7 +1393,7 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e
return err
}

tbInfo, err := buildTableInfoWithCheck(ctx, d, s, schema.Charset)
tbInfo, err := buildTableInfoWithCheck(ctx, d, s, schema.Charset, schema.Collate)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -1744,8 +1753,8 @@ func (d *ddl) handleAutoIncID(tbInfo *model.TableInfo, schemaID int64) error {
return nil
}

func resolveDefaultTableCharsetAndCollation(tbInfo *model.TableInfo, dbCharset string) (err error) {
chr, collate, err := ResolveCharsetCollation(tbInfo.Charset, dbCharset)
func resolveDefaultTableCharsetAndCollation(tbInfo *model.TableInfo, dbCharset, dbCollate string) (err error) {
chr, collate, err := ResolveCharsetCollation(tbInfo.Charset, tbInfo.Collate, dbCharset, dbCollate)
if err != nil {
return errors.Trace(err)
}
Expand All @@ -1759,18 +1768,30 @@ func resolveDefaultTableCharsetAndCollation(tbInfo *model.TableInfo, dbCharset s
return
}

func findTableOptionCharset(options []*ast.TableOption) string {
var tableCharset string
func findTableOptionCharsetAndCollate(options []*ast.TableOption) (tableCharset, tableCollate string) {
var findCnt int
for i := len(options) - 1; i >= 0; i-- {
op := options[i]
if op.Tp == ast.TableOptionCharset {
if len(tableCharset) == 0 && op.Tp == ast.TableOptionCharset {
// find the last one.
tableCharset = op.StrValue
break
findCnt++
if findCnt == 2 {
break
}
continue
}
if len(tableCollate) == 0 && op.Tp == ast.TableOptionCollate {
// find the last one.
tableCollate = op.StrValue
findCnt++
if findCnt == 2 {
break
}
continue
}
}

return tableCharset
return tableCharset, tableCollate
}

// handleTableOptions updates tableInfo according to table options.
Expand Down Expand Up @@ -2170,7 +2191,7 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab
// Ignore table constraints now, maybe return error later.
// We use length(t.Cols()) as the default offset firstly, we will change the
// column's offset later.
col, _, err = buildColumnAndConstraint(ctx, len(t.Cols()), specNewColumn, nil, t.Meta().Charset, schema.Charset)
col, _, err = buildColumnAndConstraint(ctx, len(t.Cols()), specNewColumn, nil, t.Meta().Charset, t.Meta().Collate, schema.Charset, schema.Collate)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -2692,7 +2713,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
// should take the collate in colDef.Option into consideration rather than handling it separately
specifiedCollates := extractCollateFromOption(specNewColumn)

err = setCharsetCollationFlenDecimal(&newCol.FieldType, specifiedCollates, t.Meta().Charset, schema.Charset)
err = setCharsetCollationFlenDecimal(&newCol.FieldType, specifiedCollates, t.Meta().Charset, t.Meta().Collate, schema.Charset, schema.Collate)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -3084,7 +3105,7 @@ func checkAlterTableCharset(tblInfo *model.TableInfo, dbInfo *model.DBInfo, toCh
if len(origCharset) == 0 {
// The table charset may be "", if the table is create in old TiDB version, such as v2.0.8.
// This DDL will update the table charset to default charset.
origCharset, origCollate, err = ResolveCharsetCollation("", dbInfo.Charset)
origCharset, origCollate, err = ResolveCharsetCollation("", "", dbInfo.Charset, dbInfo.Collate)
if err != nil {
return doNothing, err
}
Expand Down
2 changes: 1 addition & 1 deletion ddl/ddl_worker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ func (s *testDDLSuite) TestCancelJob(c *C) {
Tp: &types.FieldType{Tp: mysql.TypeLonglong},
Options: []*ast.ColumnOption{},
}
col, _, err := buildColumnAndConstraint(ctx, 2, newColumnDef, nil, mysql.DefaultCharset, mysql.DefaultCharset)
col, _, err := buildColumnAndConstraint(ctx, 2, newColumnDef, nil, mysql.DefaultCharset, "", mysql.DefaultCharset, "")
c.Assert(err, IsNil)

addColumnArgs := []interface{}{col, &ast.ColumnPosition{Tp: ast.ColumnPositionNone}, 0}
Expand Down
4 changes: 2 additions & 2 deletions ddl/mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (dr *mockDelRange) clear() {}

// MockTableInfo mocks a table info by create table stmt ast and a specified table id.
func MockTableInfo(ctx sessionctx.Context, stmt *ast.CreateTableStmt, tableID int64) (*model.TableInfo, error) {
cols, newConstraints, err := buildColumnsAndConstraints(ctx, stmt.Cols, stmt.Constraints, "", "")
cols, newConstraints, err := buildColumnsAndConstraints(ctx, stmt.Cols, stmt.Constraints, "", "", "", "")
if err != nil {
return nil, errors.Trace(err)
}
Expand All @@ -164,7 +164,7 @@ func MockTableInfo(ctx sessionctx.Context, stmt *ast.CreateTableStmt, tableID in
return nil, errors.Trace(err)
}

if err = resolveDefaultTableCharsetAndCollation(tbl, ""); err != nil {
if err = resolveDefaultTableCharsetAndCollation(tbl, "", ""); err != nil {
return nil, errors.Trace(err)
}

Expand Down

0 comments on commit 8d545a1

Please sign in to comment.