From af00e81dd25c0a51226745e063b112bc49262f5f Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 2 Aug 2018 14:43:35 +0800 Subject: [PATCH 1/7] fix bit default value bug --- ddl/db_test.go | 8 ++++++++ ddl/ddl_api.go | 24 +++++++++++++++++------- executor/show.go | 5 +++-- model/model.go | 21 +++++++++++++++++++++ table/column.go | 4 ++-- table/tables/tables.go | 2 +- 6 files changed, 52 insertions(+), 12 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index eab38babeebef..e27ee6f573e44 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1578,6 +1578,14 @@ func (s *testDBSuite) TestCreateTable(c *C) { c.Assert(err, NotNil) } +func (s *testDBSuite) TestBitDefaultValue(c *C) { + s.tk.MustExec("use test") + s.tk.MustExec("create table t_bit (c1 bit(10) default 250, c2 int);") + s.tk.MustExec("insert into t_bit set c2=1;") + s.tk.MustQuery("select bin(c1),c2 from t_bit").Check(testkit.Rows("11111010 1")) + s.tk.MustExec("drop table t_bit") +} + func (s *testDBSuite) TestCreateTableWithPartition(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 1f2533187c31a..a9c460046ce04 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -329,7 +329,9 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o if err = checkColumnCantHaveDefaultValue(col, value); err != nil { return nil, nil, errors.Trace(err) } - col.DefaultValue = value + if err = col.SetDefaultValue(value); err != nil { + return nil, nil, errors.Trace(err) + } hasDefaultValue = true removeOnUpdateNowFlag(col) case ast.ColumnOptionOnUpdate: @@ -485,7 +487,7 @@ func checkDefaultValue(ctx sessionctx.Context, c *table.Column, hasDefaultValue return nil } - if c.DefaultValue != nil { + if c.GetDefaultValue() != nil { if _, err := table.GetColDefaultValue(ctx, c.ToInfo()); err != nil { return types.ErrInvalidDefault.GenByArgs(c.Name) } @@ -507,7 +509,7 @@ func checkDefaultValue(ctx sessionctx.Context, c *table.Column, hasDefaultValue // checkPriKeyConstraint check all parts of a PRIMARY KEY must be NOT NULL func checkPriKeyConstraint(col *table.Column, hasDefaultValue, hasNullFlag bool, outPriKeyConstraint *ast.Constraint) error { // Primary key should not be null. - if mysql.HasPriKeyFlag(col.Flag) && hasDefaultValue && col.DefaultValue == nil { + if mysql.HasPriKeyFlag(col.Flag) && hasDefaultValue && col.GetDefaultValue() == nil { return types.ErrInvalidDefault.GenByArgs(col.Name) } // Set primary key flag for outer primary key constraint. @@ -1225,7 +1227,7 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab if err != nil { return errors.Trace(err) } - col.OriginDefaultValue = col.DefaultValue + col.OriginDefaultValue = col.GetDefaultValue() if col.OriginDefaultValue == nil && mysql.HasNotNullFlag(col.Flag) { zeroVal := table.GetZeroValue(col.ToInfo()) col.OriginDefaultValue, err = zeroVal.ToString() @@ -1436,7 +1438,10 @@ func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.Colu if err != nil { return ErrColumnBadNull.Gen("invalid default value - %s", err) } - col.DefaultValue = value + err = col.SetDefaultValue(value) + if err != nil { + return errors.Trace(err) + } return errors.Trace(checkDefaultValue(ctx, col, true)) } @@ -1465,7 +1470,9 @@ func setDefaultAndComment(ctx sessionctx.Context, col *table.Column, options []* if err = checkColumnCantHaveDefaultValue(col, value); err != nil { return errors.Trace(err) } - col.DefaultValue = value + if err = col.SetDefaultValue(value); err != nil { + return errors.Trace(err) + } hasDefaultValue = true case ast.ColumnOptionComment: err := setColumnComment(ctx, col, opt) @@ -1688,7 +1695,10 @@ func (d *ddl) AlterColumn(ctx sessionctx.Context, ident ast.Ident, spec *ast.Alt // Clean the NoDefaultValueFlag value. col.Flag &= ^mysql.NoDefaultValueFlag if len(specNewColumn.Options) == 0 { - col.DefaultValue = nil + err = col.SetDefaultValue(nil) + if err != nil { + return errors.Trace(err) + } setNoDefaultValueFlag(col, false) } else { err = setDefaultValue(ctx, col, specNewColumn.Options[0]) diff --git a/executor/show.go b/executor/show.go index 6f074811dedd9..92289dd9efb0f 100644 --- a/executor/show.go +++ b/executor/show.go @@ -503,7 +503,8 @@ func (e *ShowExec) fetchShowCreateTable() error { buf.WriteString(" NOT NULL") } if !mysql.HasNoDefaultValueFlag(col.Flag) { - switch col.DefaultValue { + defaultValue := col.GetDefaultValue() + switch defaultValue { case nil: if !mysql.HasNotNullFlag(col.Flag) { if col.Tp == mysql.TypeTimestamp { @@ -514,7 +515,7 @@ func (e *ShowExec) fetchShowCreateTable() error { case "CURRENT_TIMESTAMP": buf.WriteString(" DEFAULT CURRENT_TIMESTAMP") default: - defaultValStr := fmt.Sprintf("%v", col.DefaultValue) + defaultValStr := fmt.Sprintf("%v", defaultValue) if col.Tp == mysql.TypeBit { defaultValBinaryLiteral := types.BinaryLiteral(defaultValStr) buf.WriteString(fmt.Sprintf(" DEFAULT %s", defaultValBinaryLiteral.ToBitLiteralString(true))) diff --git a/model/model.go b/model/model.go index 49ea950705d9a..c7e35e73a6bce 100644 --- a/model/model.go +++ b/model/model.go @@ -66,6 +66,7 @@ type ColumnInfo struct { Offset int `json:"offset"` OriginDefaultValue interface{} `json:"origin_default"` DefaultValue interface{} `json:"default"` + DefaultValueBit []byte `json:"default_bit"` GeneratedExprString string `json:"generated_expr_string"` GeneratedStored bool `json:"generated_stored"` Dependences map[string]struct{} `json:"dependences"` @@ -85,6 +86,26 @@ func (c *ColumnInfo) IsGenerated() bool { return len(c.GeneratedExprString) != 0 } +func (c *ColumnInfo) SetDefaultValue(value interface{}) error { + c.DefaultValue = value + if c.Tp == mysql.TypeBit { + if v, ok := value.(string); ok { + c.DefaultValueBit = []byte(v) + return nil + } + return types.ErrInvalidDefault.GenByArgs(c.Name) + + } + return nil +} + +func (c *ColumnInfo) GetDefaultValue() interface{} { + if c.Tp == mysql.TypeBit && c.DefaultValueBit != nil { + return string(c.DefaultValueBit) + } + return c.DefaultValue +} + // FindColumnInfo finds ColumnInfo in cols by name. func FindColumnInfo(cols []*ColumnInfo, name string) *ColumnInfo { name = strings.ToLower(name) diff --git a/table/column.go b/table/column.go index c111c0fa9e589..e3766a688816e 100644 --- a/table/column.go +++ b/table/column.go @@ -227,7 +227,7 @@ func NewColDesc(col *Column) *ColDesc { } var defaultValue interface{} if !mysql.HasNoDefaultValueFlag(col.Flag) { - defaultValue = col.DefaultValue + defaultValue = col.GetDefaultValue() } extra := "" @@ -310,7 +310,7 @@ func GetColOriginDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo) (ty // GetColDefaultValue gets default value of the column. func GetColDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo) (types.Datum, error) { - return getColDefaultValue(ctx, col, col.DefaultValue) + return getColDefaultValue(ctx, col, col.GetDefaultValue()) } func getColDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo, defaultVal interface{}) (types.Datum, error) { diff --git a/table/tables/tables.go b/table/tables/tables.go index e111170aaeee1..4e9c26e03aa3d 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -919,7 +919,7 @@ func CanSkip(info *model.TableInfo, col *table.Column, value types.Datum) bool { if col.IsPKHandleColumn(info) { return true } - if col.DefaultValue == nil && value.IsNull() { + if col.GetDefaultValue() == nil && value.IsNull() { return true } if col.IsGenerated() && !col.GeneratedStored { From d8e001dc30b01c2312913c8cf7cd15a69912e2f9 Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 2 Aug 2018 15:05:59 +0800 Subject: [PATCH 2/7] update test --- ddl/db_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index e27ee6f573e44..65732a6ecc577 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1579,11 +1579,12 @@ func (s *testDBSuite) TestCreateTable(c *C) { } func (s *testDBSuite) TestBitDefaultValue(c *C) { - s.tk.MustExec("use test") - s.tk.MustExec("create table t_bit (c1 bit(10) default 250, c2 int);") - s.tk.MustExec("insert into t_bit set c2=1;") - s.tk.MustQuery("select bin(c1),c2 from t_bit").Check(testkit.Rows("11111010 1")) - s.tk.MustExec("drop table t_bit") + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create table t_bit (c1 bit(10) default 250, c2 int);") + tk.MustExec("insert into t_bit set c2=1;") + tk.MustQuery("select bin(c1),c2 from t_bit").Check(testkit.Rows("11111010 1")) + tk.MustExec("drop table t_bit") } func (s *testDBSuite) TestCreateTableWithPartition(c *C) { From c1503ebeef921314208669c89b2ee6796ff473e5 Mon Sep 17 00:00:00 2001 From: crazycs Date: Mon, 6 Aug 2018 13:41:45 +0800 Subject: [PATCH 3/7] add comment --- model/model.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/model/model.go b/model/model.go index 099733a92cd2a..a323d9994be9b 100644 --- a/model/model.go +++ b/model/model.go @@ -88,6 +88,7 @@ func (c *ColumnInfo) IsGenerated() bool { return len(c.GeneratedExprString) != 0 } +// SetDefaultValue sets the default value. func (c *ColumnInfo) SetDefaultValue(value interface{}) error { c.DefaultValue = value if c.Tp == mysql.TypeBit { @@ -101,6 +102,7 @@ func (c *ColumnInfo) SetDefaultValue(value interface{}) error { return nil } +// GetDefaultValue gets the default value. func (c *ColumnInfo) GetDefaultValue() interface{} { if c.Tp == mysql.TypeBit && c.DefaultValueBit != nil { return string(c.DefaultValueBit) From f2b8579856df1c89a26b8ac9148e3d5689819cc6 Mon Sep 17 00:00:00 2001 From: crazycs Date: Mon, 6 Aug 2018 17:04:42 +0800 Subject: [PATCH 4/7] refine code --- model/model.go | 1 - 1 file changed, 1 deletion(-) diff --git a/model/model.go b/model/model.go index a323d9994be9b..e8da644242067 100644 --- a/model/model.go +++ b/model/model.go @@ -97,7 +97,6 @@ func (c *ColumnInfo) SetDefaultValue(value interface{}) error { return nil } return types.ErrInvalidDefault.GenByArgs(c.Name) - } return nil } From 7ce59cdfaa3c0102c9169d67884b88ec60052bcd Mon Sep 17 00:00:00 2001 From: crazycs Date: Thu, 23 Aug 2018 10:44:00 +0800 Subject: [PATCH 5/7] address comment --- model/model.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/model/model.go b/model/model.go index 236b85a5e46c1..c86caa472ee3e 100644 --- a/model/model.go +++ b/model/model.go @@ -101,7 +101,9 @@ func (c *ColumnInfo) SetDefaultValue(value interface{}) error { return nil } -// GetDefaultValue gets the default value. +// GetDefaultValue gets the default value of the column. +// Default value use to stored in DefaultValue field, but now, +// bit type default value will store in DefaultValueBit for fix bit default value decode/encode bug. func (c *ColumnInfo) GetDefaultValue() interface{} { if c.Tp == mysql.TypeBit && c.DefaultValueBit != nil { return string(c.DefaultValueBit) From a5d4989d4910474dd51319c898911bb5ef957ffc Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 28 Aug 2018 16:31:54 +0800 Subject: [PATCH 6/7] address comment --- model/model.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/model/model.go b/model/model.go index 94aed3e9cb486..2fe3c7b6ebb00 100644 --- a/model/model.go +++ b/model/model.go @@ -22,6 +22,7 @@ import ( "github.com/juju/errors" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tipb/go-tipb" ) @@ -93,6 +94,8 @@ func (c *ColumnInfo) IsGenerated() bool { func (c *ColumnInfo) SetDefaultValue(value interface{}) error { c.DefaultValue = value if c.Tp == mysql.TypeBit { + // For mysql.TypeBit type, the default value storage format must be a string. + // Other value such as int must convert to string format first. if v, ok := value.(string); ok { c.DefaultValueBit = []byte(v) return nil @@ -107,7 +110,7 @@ func (c *ColumnInfo) SetDefaultValue(value interface{}) error { // bit type default value will store in DefaultValueBit for fix bit default value decode/encode bug. func (c *ColumnInfo) GetDefaultValue() interface{} { if c.Tp == mysql.TypeBit && c.DefaultValueBit != nil { - return string(c.DefaultValueBit) + return hack.String(c.DefaultValueBit) } return c.DefaultValue } From a3304059d1f4649cac81c99c2b80a27be91d67f6 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 29 Aug 2018 10:58:42 +0800 Subject: [PATCH 7/7] address comment --- ddl/ddl_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index d5e4f07029a89..5a1eaf9331768 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -475,9 +475,9 @@ func setTimestampDefaultValue(c *table.Column, hasDefaultValue bool, setOnUpdate // For timestamp Col, if is not set default value or not set null, use current timestamp. if mysql.HasTimestampFlag(c.Flag) && mysql.HasNotNullFlag(c.Flag) { if setOnUpdateNow { - c.DefaultValue = types.ZeroDatetimeStr + c.SetDefaultValue(types.ZeroDatetimeStr) } else { - c.DefaultValue = strings.ToUpper(ast.CurrentTimestamp) + c.SetDefaultValue(strings.ToUpper(ast.CurrentTimestamp)) } } }