From 3c0ed92f12f1f9df805acca91dfde7cb4f77e818 Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 18 Sep 2019 19:23:06 +0800 Subject: [PATCH 1/5] ddl: fix the issue when the set type default value is int type --- ddl/db_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ ddl/ddl_api.go | 25 +++++++++++++++++++------ go.mod | 1 + 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 8e4b214e2ce75..be8de9985bef6 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1832,6 +1832,46 @@ func (s *testDBSuite1) TestCreateTable(c *C) { c.Assert(err.Error(), Equals, "[types:1291]Column 'a' has duplicated value 'B' in ENUM") } +func (s *testDBSuite2) TestCreateTableWithSetCol(c *C) { + s.tk = testkit.NewTestKitWithInit(c, s.store) + s.tk.MustExec("create table t_set (a int, b set('e') default '');") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` int(11) DEFAULT NULL,\n" + + " `b` set('e') DEFAULT ''\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + + // The type of default value is int. + // for failure cases + s.tk.MustExec("drop table t_set") + failedSQL := "create table t_set (a set('1', '4', '10') default 0);" + s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) + failedSQL = "create table t_set (a set('1', '4', '10') default 8);" + s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) + + // for successful cases + s.tk.MustExec("create table t_set (a set('1', '4', '10', '21') default 1);") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` set('1','4','10','21') DEFAULT '1'\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + s.tk.MustExec("drop table t_set") + s.tk.MustExec("create table t_set (a set('1', '4', '10', '21') default 2);") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` set('1','4','10','21') DEFAULT '4'\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + s.tk.MustExec("drop table t_set") + s.tk.MustExec("create table t_set (a set('1', '4', '10', '21') default 3);") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` set('1','4','10','21') DEFAULT '1,4'\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + s.tk.MustExec("drop table t_set") + s.tk.MustExec("create table t_set (a set('1', '4', '10', '21') default 15);") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` set('1','4','10','21') DEFAULT '1,4,10,21'\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + s.tk.MustExec("insert into t_set value()") + s.tk.MustQuery("select * from t_set").Check(testkit.Rows("1,4,10,21")) +} + func (s *testDBSuite2) TestTableForeignKey(c *C) { s.tk = testkit.NewTestKit(c, s.store) s.tk.MustExec("use test") diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 20d0d1a67f7f0..3a149d5534b0e 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -620,8 +620,8 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o return col, constraints, nil } -func getDefaultValue(ctx sessionctx.Context, colName string, c *ast.ColumnOption, t *types.FieldType) (interface{}, error) { - tp, fsp := t.Tp, t.Decimal +func getDefaultValue(ctx sessionctx.Context, col *table.Column, c *ast.ColumnOption) (interface{}, error) { + tp, fsp := col.FieldType.Tp, col.FieldType.Decimal if tp == mysql.TypeTimestamp || tp == mysql.TypeDatetime { switch x := c.Expr.(type) { case *ast.FuncCallExpr: @@ -633,14 +633,14 @@ func getDefaultValue(ctx sessionctx.Context, colName string, c *ast.ColumnOption } } if defaultFsp != fsp { - return nil, ErrInvalidDefaultValue.GenWithStackByArgs(colName) + return nil, ErrInvalidDefaultValue.GenWithStackByArgs(col.Name.O) } } } vd, err := expression.GetTimeValue(ctx, c.Expr, tp, int8(fsp)) value := vd.GetValue() if err != nil { - return nil, ErrInvalidDefaultValue.GenWithStackByArgs(colName) + return nil, ErrInvalidDefaultValue.GenWithStackByArgs(col.Name.O) } // Value is nil means `default null`. @@ -680,10 +680,23 @@ func getDefaultValue(ctx sessionctx.Context, colName string, c *ast.ColumnOption } return strconv.FormatUint(value, 10), nil } + if tp == mysql.TypeSet && v.Kind() == types.KindInt64 { + setCnt := len(col.Elems) + maxLimit := int64(1< maxLimit { + return "", ErrInvalidDefaultValue.GenWithStackByArgs(col.Name.O) + } + setVal, err := types.ParseSetValue(col.Elems, uint64(val)) + if err != nil { + return "", errors.Trace(err) + } + v.SetMysqlSet(setVal) + } if tp == mysql.TypeDuration { var err error - if v, err = v.ConvertTo(ctx.GetSessionVars().StmtCtx, t); err != nil { + if v, err = v.ConvertTo(ctx.GetSessionVars().StmtCtx, &col.FieldType); err != nil { return "", errors.Trace(err) } } @@ -2491,7 +2504,7 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error { func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) (bool, error) { hasDefaultValue := false - value, err := getDefaultValue(ctx, col.Name.L, option, &col.FieldType) + value, err := getDefaultValue(ctx, col, option) if err != nil { return hasDefaultValue, errors.Trace(err) } diff --git a/go.mod b/go.mod index 51f5e84f294b4..ad786c9d04b7f 100644 --- a/go.mod +++ b/go.mod @@ -70,6 +70,7 @@ require ( golang.org/x/text v0.3.2 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect golang.org/x/tools v0.0.0-20190911022129-16c5e0f7d110 + google.golang.org/appengine v1.4.0 // indirect google.golang.org/genproto v0.0.0-20190905072037-92dd089d5514 // indirect google.golang.org/grpc v1.23.0 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect From 89ac601f8648640ec964a29f8d65b2d35c2cfdd1 Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 18 Sep 2019 20:11:30 +0800 Subject: [PATCH 2/5] ddl: check set default value is string type --- ddl/db_test.go | 21 ++++++++++++++---- ddl/ddl_api.go | 58 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index be8de9985bef6..49371d0e8d840 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1839,16 +1839,29 @@ func (s *testDBSuite2) TestCreateTableWithSetCol(c *C) { " `a` int(11) DEFAULT NULL,\n" + " `b` set('e') DEFAULT ''\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + s.tk.MustExec("drop table t_set") + s.tk.MustExec("create table t_set (a set('a', 'b', 'c', 'd') default 'a,C,c');") + s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + + " `a` set('a','b','c','d') DEFAULT 'a,c'\n" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) - // The type of default value is int. - // for failure cases + // It's for failure cases. + // The type of default value is string. s.tk.MustExec("drop table t_set") - failedSQL := "create table t_set (a set('1', '4', '10') default 0);" + failedSQL := "create table t_set (a set('1', '4', '10') default '3');" + s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) + failedSQL = "create table t_set (a set('1', '4', '10') default '1,4,11');" + s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) + failedSQL = "create table t_set (a set('1', '4', '10') default '1 ,4');" + s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) + // The type of default value is int. + failedSQL = "create table t_set (a set('1', '4', '10') default 0);" s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) failedSQL = "create table t_set (a set('1', '4', '10') default 8);" s.tk.MustGetErrCode(failedSQL, tmysql.ErrInvalidDefault) - // for successful cases + // The type of default value is int. + // It's for successful cases s.tk.MustExec("create table t_set (a set('1', '4', '10', '21') default 1);") s.tk.MustQuery("show create table t_set").Check(testkit.Rows("t_set CREATE TABLE `t_set` (\n" + " `a` set('1','4','10','21') DEFAULT '1'\n" + diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 3a149d5534b0e..fd6fa22a9d449 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -680,7 +680,26 @@ func getDefaultValue(ctx sessionctx.Context, col *table.Column, c *ast.ColumnOpt } return strconv.FormatUint(value, 10), nil } - if tp == mysql.TypeSet && v.Kind() == types.KindInt64 { + + switch tp { + case mysql.TypeSet: + return setSetDefaultValue(v, col) + case mysql.TypeDuration: + if v, err = v.ConvertTo(ctx.GetSessionVars().StmtCtx, &col.FieldType); err != nil { + return "", errors.Trace(err) + } + case mysql.TypeBit: + if v.Kind() == types.KindInt64 || v.Kind() == types.KindUint64 { + // For BIT fields, convert int into BinaryLiteral. + return types.NewBinaryLiteralFromUint(v.GetUint64(), -1).ToString(), nil + } + } + + return v.ToString() +} + +func setSetDefaultValue(v types.Datum, col *table.Column) (string, error) { + if v.Kind() == types.KindInt64 { setCnt := len(col.Elems) maxLimit := int64(1< Date: Wed, 18 Sep 2019 23:01:17 +0800 Subject: [PATCH 3/5] executor: update tests --- executor/seqtest/seq_executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/seqtest/seq_executor_test.go b/executor/seqtest/seq_executor_test.go index c10ac5d426f38..391efeb5d43da 100644 --- a/executor/seqtest/seq_executor_test.go +++ b/executor/seqtest/seq_executor_test.go @@ -587,7 +587,7 @@ func (s *seqTestSuite) TestShow(c *C) { "c4|varchar(6)|YES||1|", "c5|varchar(6)|YES||'C6'|", "c6|enum('s','m','l','xl')|YES||xl|", - "c7|set('a','b','c','d')|YES||a,c,c|", + "c7|set('a','b','c','d')|YES||a,c|", "c8|datetime|YES||CURRENT_TIMESTAMP|DEFAULT_GENERATED on update CURRENT_TIMESTAMP", "c9|year(4)|YES||2014|", )) From e16ae08d93a6e922c2ae11917abdced76538ac9f Mon Sep 17 00:00:00 2001 From: Lynn Date: Tue, 24 Sep 2019 14:09:46 +0800 Subject: [PATCH 4/5] *: address comments --- ddl/ddl_api.go | 1 + go.mod | 1 - planner/core/preprocess.go | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index fd6fa22a9d449..235fb3d0494e2 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -698,6 +698,7 @@ func getDefaultValue(ctx sessionctx.Context, col *table.Column, c *ast.ColumnOpt return v.ToString() } +// setSetDefaultValue sets the default value for the set type. See https://dev.mysql.com/doc/refman/5.7/en/set.html. func setSetDefaultValue(v types.Datum, col *table.Column) (string, error) { if v.Kind() == types.KindInt64 { setCnt := len(col.Elems) diff --git a/go.mod b/go.mod index ad786c9d04b7f..51f5e84f294b4 100644 --- a/go.mod +++ b/go.mod @@ -70,7 +70,6 @@ require ( golang.org/x/text v0.3.2 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect golang.org/x/tools v0.0.0-20190911022129-16c5e0f7d110 - google.golang.org/appengine v1.4.0 // indirect google.golang.org/genproto v0.0.0-20190905072037-92dd089d5514 // indirect google.golang.org/grpc v1.23.0 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 533195ce83c2f..1336055068cd9 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -614,7 +614,7 @@ func checkColumn(colDef *ast.ColumnDef) error { if len(tp.Elems) > mysql.MaxTypeSetMembers { return types.ErrTooBigSet.GenWithStack("Too many strings for column %s and SET", colDef.Name.Name.O) } - // Check set elements. See https://dev.mysql.com/doc/refman/5.7/en/set.html . + // Check set elements. See https://dev.mysql.com/doc/refman/5.7/en/set.html. for _, str := range colDef.Tp.Elems { if strings.Contains(str, ",") { return types.ErrIllegalValueForType.GenWithStackByArgs(types.TypeStr(tp.Tp), str) From 222d0d5508c54edd0d092fd21f19e08a67248503 Mon Sep 17 00:00:00 2001 From: Lynn Date: Thu, 26 Sep 2019 11:38:42 +0800 Subject: [PATCH 5/5] ddl: address a comment --- ddl/ddl_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 235fb3d0494e2..0aa9e80b67c12 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -724,7 +724,7 @@ func setSetDefaultValue(v types.Datum, col *table.Column) (string, error) { } valMap := make(map[string]struct{}, len(col.Elems)) - dVals := strings.SplitN(strings.ToLower(str), ",", -1) + dVals := strings.Split(strings.ToLower(str), ",") for _, dv := range dVals { valMap[dv] = struct{}{} }