From cff955cad48911cfda00a80c5bbba08ede04d7d4 Mon Sep 17 00:00:00 2001 From: winkyao Date: Tue, 22 Jan 2019 10:32:57 +0800 Subject: [PATCH 1/2] ddl: resolve the charset by the order: table->database->server (#9105) --- ddl/column_test.go | 2 +- ddl/db_integration_test.go | 31 ++++++++++ ddl/ddl_api.go | 116 ++++++++++++++++++++++++++----------- ddl/ddl_worker_test.go | 2 +- ddl/mock.go | 11 +++- server/driver_tidb.go | 9 ++- 6 files changed, 132 insertions(+), 39 deletions(-) diff --git a/ddl/column_test.go b/ddl/column_test.go index 6a66cbfbf94de..0da79df1c9087 100644 --- a/ddl/column_test.go +++ b/ddl/column_test.go @@ -953,7 +953,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) + col, _, err := buildColumnAndConstraint(nil, 0, colDef, nil, "", "") c.Assert(err, IsNil) return &col.FieldType } diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index b6a0da7018612..69e37e4c21453 100644 --- a/ddl/db_integration_test.go +++ b/ddl/db_integration_test.go @@ -18,6 +18,7 @@ import ( . "github.com/pingcap/check" "github.com/pingcap/errors" + "github.com/pingcap/parser/model" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" @@ -243,3 +244,33 @@ func newStoreWithBootstrap() (kv.Storage, *domain.Domain, error) { dom, err := session.BootstrapSession(store) return store, dom, errors.Trace(err) } + +func (s *testIntegrationSuite) TestResolveCharset(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists resolve_charset") + tk.MustExec(`CREATE TABLE resolve_charset (a varchar(255) DEFAULT NULL) DEFAULT CHARSET=latin1`) + ctx := tk.Se.(sessionctx.Context) + is := domain.GetDomain(ctx).InfoSchema() + tbl, err := is.TableByName(model.NewCIStr("test"), model.NewCIStr("resolve_charset")) + c.Assert(err, IsNil) + c.Assert(tbl.Cols()[0].Charset, Equals, "latin1") + tk.MustExec("INSERT INTO resolve_charset VALUES('鰈')") + + tk.MustExec("create database resolve_charset charset binary") + tk.MustExec("use resolve_charset") + tk.MustExec(`CREATE TABLE resolve_charset (a varchar(255) DEFAULT NULL) DEFAULT CHARSET=latin1`) + + is = domain.GetDomain(ctx).InfoSchema() + tbl, err = is.TableByName(model.NewCIStr("resolve_charset"), model.NewCIStr("resolve_charset")) + c.Assert(err, IsNil) + c.Assert(tbl.Cols()[0].Charset, Equals, "latin1") + c.Assert(tbl.Meta().Charset, Equals, "latin1") + + tk.MustExec(`CREATE TABLE resolve_charset1 (a varchar(255) DEFAULT NULL)`) + is = domain.GetDomain(ctx).InfoSchema() + tbl, err = is.TableByName(model.NewCIStr("resolve_charset"), model.NewCIStr("resolve_charset1")) + c.Assert(err, IsNil) + c.Assert(tbl.Cols()[0].Charset, Equals, "binary") + c.Assert(tbl.Meta().Charset, Equals, "binary") +} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index c839b5922d994..3d2fc2120f3b3 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -163,7 +163,7 @@ func setColumnFlagWithConstraint(colMap map[string]*table.Column, v *ast.Constra } func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef, - constraints []*ast.Constraint) ([]*table.Column, []*ast.Constraint, error) { + constraints []*ast.Constraint, tblCharset, dbCharset string) ([]*table.Column, []*ast.Constraint, error) { var cols []*table.Column 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)); @@ -175,7 +175,7 @@ func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef } } for i, colDef := range colDefs { - col, cts, err := buildColumnAndConstraint(ctx, i, colDef, outPriKeyConstraint) + col, cts, err := buildColumnAndConstraint(ctx, i, colDef, outPriKeyConstraint, tblCharset, dbCharset) if err != nil { return nil, nil, errors.Trace(err) } @@ -191,13 +191,40 @@ func buildColumnsAndConstraints(ctx sessionctx.Context, colDefs []*ast.ColumnDef return cols, constraints, nil } -func setCharsetCollationFlenDecimal(tp *types.FieldType) error { +// ResolveCharsetCollation will resolve the charset by the order: table charset > database charset > server default charset. +func ResolveCharsetCollation(tblCharset, dbCharset string) (string, string, error) { + if len(tblCharset) != 0 { + defCollate, err := charset.GetDefaultCollation(tblCharset) + if err != nil { + // return terror is better. + return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(tblCharset) + } + return tblCharset, defCollate, nil + } + + if len(dbCharset) != 0 { + defCollate, err := charset.GetDefaultCollation(dbCharset) + if err != nil { + return "", "", ErrUnknownCharacterSet.GenWithStackByArgs(dbCharset) + } + return dbCharset, defCollate, errors.Trace(err) + } + + charset, collate := charset.GetDefaultCharsetAndCollate() + return charset, collate, nil +} + +func setCharsetCollationFlenDecimal(tp *types.FieldType, tblCharset string, dbCharset string) error { tp.Charset = strings.ToLower(tp.Charset) tp.Collate = strings.ToLower(tp.Collate) if len(tp.Charset) == 0 { switch tp.Tp { case mysql.TypeString, mysql.TypeVarchar, mysql.TypeVarString, mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob, mysql.TypeEnum, mysql.TypeSet: - tp.Charset, tp.Collate = charset.GetDefaultCharsetAndCollate() + var err error + tp.Charset, tp.Collate, err = ResolveCharsetCollation(tblCharset, dbCharset) + if err != nil { + return errors.Trace(err) + } default: tp.Charset = charset.CharsetBin tp.Collate = charset.CharsetBin @@ -232,8 +259,8 @@ func setCharsetCollationFlenDecimal(tp *types.FieldType) error { // 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) (*table.Column, []*ast.Constraint, error) { - err := setCharsetCollationFlenDecimal(colDef.Tp) + colDef *ast.ColumnDef, outPriKeyConstraint *ast.Constraint, tblCharset, dbCharset string) (*table.Column, []*ast.Constraint, error) { + err := setCharsetCollationFlenDecimal(colDef.Tp, tblCharset, dbCharset) if err != nil { return nil, nil, errors.Trace(err) } @@ -613,25 +640,23 @@ func checkColumnsAttributes(colDefs []*ast.ColumnDef) error { return nil } -// checkColumnFieldLength check the maximum length limit for different character set varchar type columns. -func checkColumnFieldLength(schema *model.DBInfo, colDefs []*ast.ColumnDef, tbInfo *model.TableInfo) error { - for _, colDef := range colDefs { - if colDef.Tp.Tp == mysql.TypeVarchar { - var setCharset string - setCharset = mysql.DefaultCharset - if len(schema.Charset) != 0 { - setCharset = schema.Charset - } - if len(tbInfo.Charset) != 0 { - setCharset = tbInfo.Charset - } +// checkColumnsFieldLength check the maximum length limit for different character set varchar type columns. +func checkColumnsFieldLength(cols []*table.Column) error { + for _, col := range cols { + if err := checkColumnFieldLength(col); err != nil { + return errors.Trace(err) + } + } + return nil +} - err := IsTooBigFieldLength(colDef.Tp.Flen, colDef.Name.Name.O, setCharset) - if err != nil { - return errors.Trace(err) - } +func checkColumnFieldLength(col *table.Column) error { + if col.Tp == mysql.TypeVarchar { + if err := IsTooBigFieldLength(col.Flen, col.Name.O, col.Charset); err != nil { + return errors.Trace(err) } } + return nil } @@ -885,6 +910,20 @@ func (d *ddl) CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast. return errors.Trace(err) } +func findTableOptionCharset(options []*ast.TableOption) string { + var tableCharset string + for i := len(options) - 1; i >= 0; i-- { + op := options[i] + if op.Tp == ast.TableOptionCharset { + // find the last one. + tableCharset = op.StrValue + break + } + } + + return tableCharset +} + func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err error) { ident := ast.Ident{Schema: s.Table.Schema, Name: s.Table.Name} if s.ReferTable != nil { @@ -924,11 +963,17 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } - cols, newConstraints, err := buildColumnsAndConstraints(ctx, colDefs, s.Constraints) + tableCharset := findTableOptionCharset(s.Options) + // The column charset haven't been resolved here. + cols, newConstraints, err := buildColumnsAndConstraints(ctx, colDefs, s.Constraints, tableCharset, schema.Charset) if err != nil { return errors.Trace(err) } + if err = checkColumnsFieldLength(cols); err != nil { + return errors.Trace(err) + } + err = checkConstraintNames(newConstraints) if err != nil { return errors.Trace(err) @@ -987,13 +1032,15 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e if err != nil { return errors.Trace(err) } + + if err = resolveDefaultTableCharsetAndCollation(tbl, ""); err != nil { + return nil, errors.Trace(err) + } + err = checkCharsetAndCollation(tbInfo.Charset, tbInfo.Collate) if err != nil { return errors.Trace(err) } - if err = checkColumnFieldLength(schema, s.Cols, tbInfo); err != nil { - return errors.Trace(err) - } err = d.doDDLJob(ctx, job) if err == nil { @@ -1037,13 +1084,17 @@ func (d *ddl) handleAutoIncID(tbInfo *model.TableInfo, schemaID int64) error { return nil } -func setDefaultTableCharsetAndCollation(tbInfo *model.TableInfo) (err error) { +func resolveDefaultTableCharsetAndCollation(tbInfo *model.TableInfo, dbCharset string) (err error) { + chr, collate, err := ResolveCharsetCollation(tbInfo.Charset, dbCharset) + if err != nil { + return errors.Trace(err) + } if len(tbInfo.Charset) == 0 { - tbInfo.Charset = mysql.DefaultCharset + tbInfo.Charset = chr } if len(tbInfo.Collate) == 0 { - tbInfo.Collate, err = charset.GetDefaultCollation(tbInfo.Charset) + tbInfo.Collate = collate } return } @@ -1073,9 +1124,6 @@ func handleTableOptions(options []*ast.TableOption, tbInfo *model.TableInfo) err } } - if err := setDefaultTableCharsetAndCollation(tbInfo); err != nil { - log.Error(errors.ErrorStack(err)) - } return nil } @@ -1355,7 +1403,7 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab // Ingore 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) + col, _, err = buildColumnAndConstraint(ctx, len(t.Cols()), specNewColumn, nil, t.Meta().Charset, schema.Charset) if err != nil { return errors.Trace(err) } @@ -1802,7 +1850,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or return nil, errUnsupportedModifyColumn.GenWithStackByArgs("null to not null") } - if err = checkColumnFieldLength(schema, spec.NewColumns, t.Meta()); err != nil { + if err = checkColumnFieldLength(newCol); err != nil { return nil, errors.Trace(err) } diff --git a/ddl/ddl_worker_test.go b/ddl/ddl_worker_test.go index d991ee929675a..c83c7c842a0e3 100644 --- a/ddl/ddl_worker_test.go +++ b/ddl/ddl_worker_test.go @@ -497,7 +497,7 @@ func (s *testDDLSuite) TestCancelJob(c *C) { Tp: &types.FieldType{Tp: mysql.TypeLonglong}, Options: []*ast.ColumnOption{}, } - col, _, err := buildColumnAndConstraint(ctx, 2, newColumnDef, nil) + col, _, err := buildColumnAndConstraint(ctx, 2, newColumnDef, nil, "", "") c.Assert(err, IsNil) addColumnArgs := []interface{}{col, &ast.ColumnPosition{Tp: ast.ColumnPositionNone}, 0} doDDLJobErrWithSchemaState(ctx, d, c, dbInfo.ID, tblInfo.ID, model.ActionAddColumn, addColumnArgs, &cancelState) diff --git a/ddl/mock.go b/ddl/mock.go index 0ba650f5a4847..8dd2a77707d10 100644 --- a/ddl/mock.go +++ b/ddl/mock.go @@ -138,7 +138,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) } @@ -147,5 +147,14 @@ func MockTableInfo(ctx sessionctx.Context, stmt *ast.CreateTableStmt, tableID in return nil, errors.Trace(err) } tbl.ID = tableID + + // The specified charset will be handled in handleTableOptions + if err = handleTableOptions(stmt.Options, tbl); err != nil { + return nil, errors.Trace(err) + } + + if err = resolveDefaultTableCharsetAndCollation(tbl, ""); err != nil { + return nil, errors.Trace(err) + } return tbl, nil } diff --git a/server/driver_tidb.go b/server/driver_tidb.go index aac7dc297211e..20991846579e1 100644 --- a/server/driver_tidb.go +++ b/server/driver_tidb.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util" + "github.com/pingcap/tidb/util/charset" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/sqlexec" "golang.org/x/net/context" @@ -409,10 +410,14 @@ func convertColumnInfo(fld *ast.ResultField) (ci *ColumnInfo) { // * gb2312, the multiple is 2 // * Utf-8, the multiple is 3 // * utf8mb4, the multiple is 4 - // So the large enough multiple is 4 in here. // We used to check non-string types to avoid the truncation problem in some MySQL // client such as Navicat. Now we only allow string type enter this branch. - ci.ColumnLength = ci.ColumnLength * mysql.MaxBytesOfCharacter + charsetDesc, err := charset.GetCharsetDesc(fld.Column.Charset) + if err != nil { + ci.ColumnLength = ci.ColumnLength * 4 + } else { + ci.ColumnLength = ci.ColumnLength * uint32(charsetDesc.Maxlen) + } } if fld.Column.Decimal == types.UnspecifiedLength { From b350c84478e90cc0e854a3cf72a4a5dfdea2d0d3 Mon Sep 17 00:00:00 2001 From: winkyao Date: Tue, 22 Jan 2019 10:46:24 +0800 Subject: [PATCH 2/2] fix ci --- ddl/ddl_api.go | 6 +++--- executor/show_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 3d2fc2120f3b3..ab692991cb286 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1033,8 +1033,8 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } - if err = resolveDefaultTableCharsetAndCollation(tbl, ""); err != nil { - return nil, errors.Trace(err) + if err = resolveDefaultTableCharsetAndCollation(tbInfo, schema.Charset); err != nil { + return errors.Trace(err) } err = checkCharsetAndCollation(tbInfo.Charset, tbInfo.Collate) @@ -1820,7 +1820,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or Name: newColName, }) - err = setCharsetCollationFlenDecimal(&newCol.FieldType) + err = setCharsetCollationFlenDecimal(&newCol.FieldType, t.Meta().Charset, schema.Charset) if err != nil { return nil, errors.Trace(err) } diff --git a/executor/show_test.go b/executor/show_test.go index efd82fc5592d1..185540adafd99 100644 --- a/executor/show_test.go +++ b/executor/show_test.go @@ -54,7 +54,7 @@ func (s *testSuite) TestShow(c *C) { row := result.Rows()[0] // For issue https://github.com/pingcap/tidb/issues/1061 expectedRow := []interface{}{ - "SHOW_test", "CREATE TABLE `SHOW_test` (\n `id` int(11) NOT NULL AUTO_INCREMENT,\n `c1` int(11) DEFAULT NULL COMMENT 'c1_comment',\n `c2` int(11) DEFAULT NULL,\n `c3` int(11) DEFAULT '1',\n `c4` text CHARSET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL,\n `c5` tinyint(1) DEFAULT NULL,\n PRIMARY KEY (`id`),\n KEY `idx_wide_c4` (`c3`,`c4`(10))\n) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin AUTO_INCREMENT=28934 COMMENT='table_comment'"} + "SHOW_test", "CREATE TABLE `SHOW_test` (\n `id` int(11) NOT NULL AUTO_INCREMENT,\n `c1` int(11) DEFAULT NULL COMMENT 'c1_comment',\n `c2` int(11) DEFAULT NULL,\n `c3` int(11) DEFAULT '1',\n `c4` text CHARSET utf8 COLLATE utf8_bin DEFAULT NULL,\n `c5` tinyint(1) DEFAULT NULL,\n PRIMARY KEY (`id`),\n KEY `idx_wide_c4` (`c3`,`c4`(10))\n) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin AUTO_INCREMENT=28934 COMMENT='table_comment'"} for i, r := range row { c.Check(r, Equals, expectedRow[i]) }