From e74e0336418933cec8a4dc89e66406c4fe37ffb6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 21 Feb 2019 14:46:22 +0800 Subject: [PATCH 1/3] * fix timestamp default value bug in multiple time zones. (#9115) --- ddl/column.go | 19 ++++++++-- ddl/ddl_api.go | 73 +++++++++++++++++++++++++++------------ executor/executor_test.go | 69 ++++++++++++++++++++++++++++++++++++ executor/show.go | 32 +++++++++++++++-- executor/write_test.go | 9 +++++ go.mod | 2 ++ go.sum | 2 ++ table/column.go | 32 +++++++++++++---- 8 files changed, 203 insertions(+), 35 deletions(-) diff --git a/ddl/column.go b/ddl/column.go index cf98630d606b9..273ba5ca62680 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -246,6 +246,12 @@ func onDropColumn(t *meta.Meta, job *model.Job) (ver int64, _ error) { // NOTE: If the state of StateWriteOnly can be rollbacked, we'd better reconsider the original default value. // And we need consider the column without not-null flag. if colInfo.OriginDefaultValue == nil && mysql.HasNotNullFlag(colInfo.Flag) { + // If the column is timestamp default current_timestamp, and DDL owner is new version TiDB that set column.Version to 1, + // then old TiDB update record in the column write only stage will uses the wrong default value of the dropping column. + // Because new version of the column default value is UTC time, but old version TiDB will think the default value is the time in system timezone. + // But currently will be ok, because we can't cancel the drop column job when the job is running, + // so the column will be dropped succeed and client will never see the wrong default value of the dropped column. + // More info about this problem, see PR#9115. colInfo.OriginDefaultValue, err = generateOriginDefaultValue(colInfo) if err != nil { return ver, errors.Trace(err) @@ -463,9 +469,16 @@ func generateOriginDefaultValue(col *model.ColumnInfo) (interface{}, error) { } } - if odValue == strings.ToUpper(ast.CurrentTimestamp) && - (col.Tp == mysql.TypeTimestamp || col.Tp == mysql.TypeDatetime) { - odValue = time.Now().Format(types.TimeFormat) + if odValue == strings.ToUpper(ast.CurrentTimestamp) { + if col.Tp == mysql.TypeTimestamp { + odValue = time.Now().UTC().Format(types.TimeFormat) + // Version = 1: For OriginDefaultValue and DefaultValue of timestamp column will stores the default time in UTC time zone. + // This will fix bug in version 0. + // TODO: remove this version field after there is no old version 0. + col.Version = model.ColumnInfoVersion1 + } else if col.Tp == mysql.TypeDatetime { + odValue = time.Now().Format(types.TimeFormat) + } } return odValue, nil } diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index bd11186d66013..b49454f53fd67 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -21,6 +21,7 @@ import ( "bytes" "fmt" "strings" + "time" "github.com/cznic/mathutil" "github.com/pingcap/errors" @@ -299,6 +300,30 @@ func checkColumnDefaultValue(ctx sessionctx.Context, col *table.Column, value in return hasDefaultValue, value, nil } +func convertTimestampDefaultValToUTC(ctx sessionctx.Context, defaultVal interface{}, col *table.Column) (interface{}, error) { + if defaultVal == nil || col.Tp != mysql.TypeTimestamp { + return defaultVal, nil + } + if vv, ok := defaultVal.(string); ok { + if vv != types.ZeroDatetimeStr && strings.ToUpper(vv) != strings.ToUpper(ast.CurrentTimestamp) { + t, err := types.ParseTime(ctx.GetSessionVars().StmtCtx, vv, col.Tp, col.Decimal) + if err != nil { + return defaultVal, errors.Trace(err) + } + err = t.ConvertTimeZone(ctx.GetSessionVars().Location(), time.UTC) + if err != nil { + return defaultVal, errors.Trace(err) + } + defaultVal = t.String() + // Version = 1: For OriginDefaultValue and DefaultValue of timestamp column will stores the default time in UTC time zone. + // This will fix bug in version 0. + // TODO: remove this version field after there is no old version 0. + col.Version = model.ColumnInfoVersion1 + } + } + return defaultVal, nil +} + // isExplicitTimeStamp is used to check if explicit_defaults_for_timestamp is on or off. // Check out this link for more details. // https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp @@ -325,7 +350,7 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o col.Flag |= mysql.NotNullFlag } } - + var err error setOnUpdateNow := false hasDefaultValue := false hasNullFlag := false @@ -358,14 +383,8 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o constraints = append(constraints, constraint) col.Flag |= mysql.UniqueKeyFlag case ast.ColumnOptionDefaultValue: - value, err := getDefaultValue(ctx, v, colDef.Tp.Tp, colDef.Tp.Decimal) + hasDefaultValue, err = setDefaultValue(ctx, col, v) if err != nil { - return nil, nil, ErrColumnBadNull.GenWithStack("invalid default value - %s", err) - } - if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil { - return nil, nil, errors.Trace(err) - } - if err = col.SetDefaultValue(value); err != nil { return nil, nil, errors.Trace(err) } removeOnUpdateNowFlag(col) @@ -422,7 +441,7 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o if mysql.HasZerofillFlag(col.Flag) { col.Flag |= mysql.UnsignedFlag } - err := checkPriKeyConstraint(col, hasDefaultValue, hasNullFlag, outPriKeyConstraint) + err = checkPriKeyConstraint(col, hasDefaultValue, hasNullFlag, outPriKeyConstraint) if err != nil { return nil, nil, errors.Trace(err) } @@ -433,7 +452,8 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o return col, constraints, nil } -func getDefaultValue(ctx sessionctx.Context, c *ast.ColumnOption, tp byte, fsp int) (interface{}, error) { +func getDefaultValue(ctx sessionctx.Context, c *ast.ColumnOption, t *types.FieldType) (interface{}, error) { + tp, fsp := t.Tp, t.Decimal if tp == mysql.TypeTimestamp || tp == mysql.TypeDatetime { vd, err := expression.GetTimeValue(ctx, c.Expr, tp, fsp) value := vd.GetValue() @@ -1673,16 +1693,25 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error { return errUnsupportedModifyColumn.GenWithStackByArgs(msg) } -func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) error { - value, err := getDefaultValue(ctx, option, col.Tp, col.Decimal) +func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) (bool, error) { + hasDefaultValue := false + value, err := getDefaultValue(ctx, option, &col.FieldType) + if err != nil { + return hasDefaultValue, ErrColumnBadNull.GenWithStack("invalid default value - %s", err) + } + + if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil { + return hasDefaultValue, errors.Trace(err) + } + value, err = convertTimestampDefaultValToUTC(ctx, value, col) if err != nil { - return ErrColumnBadNull.GenWithStack("invalid default value - %s", err) + return hasDefaultValue, errors.Trace(err) } err = col.SetDefaultValue(value) if err != nil { - return errors.Trace(err) + return hasDefaultValue, errors.Trace(err) } - return errors.Trace(checkDefaultValue(ctx, col, true)) + return hasDefaultValue, nil } func setColumnComment(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) error { @@ -1700,17 +1729,12 @@ func setDefaultAndComment(ctx sessionctx.Context, col *table.Column, options []* return nil } var hasDefaultValue, setOnUpdateNow bool + var err error for _, opt := range options { switch opt.Tp { case ast.ColumnOptionDefaultValue: - value, err := getDefaultValue(ctx, opt, col.Tp, col.Decimal) + hasDefaultValue, err = setDefaultValue(ctx, col, opt) if err != nil { - return ErrColumnBadNull.GenWithStack("invalid default value - %s", err) - } - if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil { - return errors.Trace(err) - } - if err = col.SetDefaultValue(value); err != nil { return errors.Trace(err) } case ast.ColumnOptionComment: @@ -1945,10 +1969,13 @@ func (d *ddl) AlterColumn(ctx sessionctx.Context, ident ast.Ident, spec *ast.Alt } setNoDefaultValueFlag(col, false) } else { - err = setDefaultValue(ctx, col, specNewColumn.Options[0]) + hasDefaultValue, err := setDefaultValue(ctx, col, specNewColumn.Options[0]) if err != nil { return errors.Trace(err) } + if err = checkDefaultValue(ctx, col, hasDefaultValue); err != nil { + return errors.Trace(err) + } } job := &model.Job{ diff --git a/executor/executor_test.go b/executor/executor_test.go index 507d94f0c134a..f78899e1e9e2d 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -37,6 +37,7 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/executor" + "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta/autoid" plannercore "github.com/pingcap/tidb/planner/core" @@ -2130,6 +2131,74 @@ func (s *testSuite) TestTimestampTimeZone(c *C) { r.Check(testkit.Rows("2014-03-31 08:57:10")) } +func (s *testSuite) TestTimestampDefaultValueTimeZone(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("set time_zone = '+08:00'") + tk.MustExec(`create table t (a int, b timestamp default "2019-01-17 14:46:14")`) + tk.MustExec("insert into t set a=1") + r := tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '2019-01-17 14:46:14'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + tk.MustExec("set time_zone = '+00:00'") + tk.MustExec("insert into t set a=2") + r = tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '2019-01-17 06:46:14'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + r = tk.MustQuery(`select a,b from t order by a`) + r.Check(testkit.Rows("1 2019-01-17 06:46:14", "2 2019-01-17 06:46:14")) + tk.MustExec("set time_zone = '+08:00'") + r = tk.MustQuery(`select a,b from t order by a`) + r.Check(testkit.Rows("1 2019-01-17 14:46:14", "2 2019-01-17 14:46:14")) + tk.MustExec("set time_zone = '-08:00'") + r = tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '2019-01-16 22:46:14'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + + // test zero default value in multiple time zone. + defer tk.MustExec(fmt.Sprintf("set @@sql_mode='%s'", tk.MustQuery("select @@sql_mode").Rows()[0][0])) + tk.MustExec("set @@sql_mode='STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION';") + tk.MustExec("drop table if exists t") + tk.MustExec("set time_zone = '+08:00'") + tk.MustExec(`create table t (a int, b timestamp default "0000-00-00 00")`) + tk.MustExec("insert into t set a=1") + r = tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '0000-00-00 00:00:00'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + tk.MustExec("set time_zone = '+00:00'") + tk.MustExec("insert into t set a=2") + r = tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '0000-00-00 00:00:00'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + tk.MustExec("set time_zone = '-08:00'") + tk.MustExec("insert into t set a=3") + r = tk.MustQuery(`show create table t`) + r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '0000-00-00 00:00:00'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) + r = tk.MustQuery(`select a,b from t order by a`) + r.Check(testkit.Rows("1 0000-00-00 00:00:00", "2 0000-00-00 00:00:00", "3 0000-00-00 00:00:00")) + + // test add timestamp column default current_timestamp. + tk.MustExec(`drop table if exists t`) + tk.MustExec(`set time_zone = 'Asia/Shanghai'`) + tk.MustExec(`create table t (a int)`) + tk.MustExec(`insert into t set a=1`) + tk.MustExec(`alter table t add column b timestamp not null default current_timestamp;`) + timeIn8 := tk.MustQuery("select b from t").Rows()[0][0] + tk.MustExec(`set time_zone = '+00:00'`) + timeIn0 := tk.MustQuery("select b from t").Rows()[0][0] + c.Assert(timeIn8 != timeIn0, IsTrue, Commentf("%v == %v", timeIn8, timeIn0)) + datumTimeIn8, err := expression.GetTimeValue(tk.Se, timeIn8, mysql.TypeTimestamp, 0) + c.Assert(err, IsNil) + tIn8To0 := datumTimeIn8.GetMysqlTime() + timeZoneIn8, err := time.LoadLocation("Asia/Shanghai") + c.Assert(err, IsNil) + err = tIn8To0.ConvertTimeZone(timeZoneIn8, time.UTC) + c.Assert(err, IsNil) + c.Assert(timeIn0 == tIn8To0.String(), IsTrue, Commentf("%v != %v", timeIn0, tIn8To0.String())) + + // test add index. + tk.MustExec(`alter table t add index(b);`) + tk.MustExec("admin check table t") + tk.MustExec(`set time_zone = '+05:00'`) + tk.MustExec("admin check table t") +} + func (s *testSuite) TestTiDBCurrentTS(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustQuery("select @@tidb_current_ts").Check(testkit.Rows("0")) diff --git a/executor/show.go b/executor/show.go index 6aa2072d8cdcc..874e70f6a7386 100644 --- a/executor/show.go +++ b/executor/show.go @@ -276,6 +276,25 @@ func (e *ShowExec) fetchShowColumns() error { } desc := table.NewColDesc(col) + var columnDefault interface{} + if desc.DefaultValue != nil { + // SHOW COLUMNS result expects string value + defaultValStr := fmt.Sprintf("%v", desc.DefaultValue) + // If column is timestamp, and default value is not current_timestamp, should convert the default value to the current session time zone. + if col.Tp == mysql.TypeTimestamp && defaultValStr != types.ZeroDatetimeStr && strings.ToUpper(defaultValStr) != strings.ToUpper(ast.CurrentTimestamp) { + timeValue, err := table.GetColDefaultValue(e.ctx, col.ToInfo()) + if err != nil { + return errors.Trace(err) + } + defaultValStr = timeValue.GetMysqlTime().String() + } + if col.Tp == mysql.TypeBit { + defaultValBinaryLiteral := types.BinaryLiteral(defaultValStr) + columnDefault = defaultValBinaryLiteral.ToBitLiteralString(true) + } else { + columnDefault = defaultValStr + } + } // The FULL keyword causes the output to include the column collation and comments, // as well as the privileges you have for each column. @@ -286,7 +305,7 @@ func (e *ShowExec) fetchShowColumns() error { desc.Collation, desc.Null, desc.Key, - desc.DefaultValue, + columnDefault, desc.Extra, desc.Privileges, desc.Comment, @@ -297,7 +316,7 @@ func (e *ShowExec) fetchShowColumns() error { desc.Type, desc.Null, desc.Key, - desc.DefaultValue, + columnDefault, desc.Extra, }) } @@ -543,6 +562,15 @@ func (e *ShowExec) fetchShowCreateTable() error { buf.WriteString(" DEFAULT CURRENT_TIMESTAMP") default: defaultValStr := fmt.Sprintf("%v", defaultValue) + // If column is timestamp, and default value is not current_timestamp, should convert the default value to the current session time zone. + if col.Tp == mysql.TypeTimestamp && defaultValStr != types.ZeroDatetimeStr { + timeValue, err := table.GetColDefaultValue(e.ctx, col.ToInfo()) + if err != nil { + return errors.Trace(err) + } + defaultValStr = timeValue.GetMysqlTime().String() + } + if col.Tp == mysql.TypeBit { defaultValBinaryLiteral := types.BinaryLiteral(defaultValStr) buf.WriteString(fmt.Sprintf(" DEFAULT %s", defaultValBinaryLiteral.ToBitLiteralString(true))) diff --git a/executor/write_test.go b/executor/write_test.go index 4f0aa412a4c44..f9a002fe628d6 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -1861,6 +1861,15 @@ func (s *testSuite) TestNullDefault(c *C) { tk.MustQuery("select * from test_null_default").Check(testkit.Rows("", "1970-01-01 08:20:34")) } +func (s *testSuite) TestNotNullDefault(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test; drop table if exists t1,t2;") + defer tk.MustExec("drop table t1,t2") + tk.MustExec("create table t1 (a int not null default null default 1);") + tk.MustExec("create table t2 (a int);") + tk.MustExec("alter table t2 change column a a int not null default null default 1;") +} + func (s *testBypassSuite) TestBypassLatch(c *C) { store, err := mockstore.NewMockTikvStore( // Small latch slot size to make conflicts. diff --git a/go.mod b/go.mod index bd6bc2dbf0957..ac536ffd669e3 100644 --- a/go.mod +++ b/go.mod @@ -81,3 +81,5 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.2.1 // indirect ) + +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190221063202-f830e9210447 diff --git a/go.sum b/go.sum index ce2691fdcb146..a21c198685268 100644 --- a/go.sum +++ b/go.sum @@ -20,6 +20,8 @@ github.com/coreos/go-systemd v0.0.0-20180202092358-40e2722dffea h1:IHPWgevPcOUjT github.com/coreos/go-systemd v0.0.0-20180202092358-40e2722dffea/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf h1:CAKfRE2YtTUIjjh1bkBtyYFaUT/WmOqsJjgtihT0vMI= github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= +github.com/crazycs520/parser v0.0.0-20190221063202-f830e9210447 h1:ZeTcpxSR4nUI9LP4HXyTiU//euTHaCewBEufdno24z0= +github.com/crazycs520/parser v0.0.0-20190221063202-f830e9210447/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20160613104831-78ad7f262603 h1:hhR9hTi0ligs11JjfGDBP332clNOJRdW0Ci5oHtEC+0= github.com/cznic/mathutil v0.0.0-20160613104831-78ad7f262603/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= diff --git a/table/column.go b/table/column.go index b4a0c8f91f7b3..bcbbe4dae965d 100644 --- a/table/column.go +++ b/table/column.go @@ -19,6 +19,7 @@ package table import ( "strings" + "time" "unicode/utf8" "github.com/pingcap/errors" @@ -33,6 +34,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/types/json" "github.com/pingcap/tidb/util/hack" + "github.com/pingcap/tidb/util/timeutil" log "github.com/sirupsen/logrus" ) @@ -350,18 +352,34 @@ func getColDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo, defaultVa return getColDefaultValueFromNil(ctx, col) } - // Check and get timestamp/datetime default value. - if col.Tp == mysql.TypeTimestamp || col.Tp == mysql.TypeDatetime { - value, err := expression.GetTimeValue(ctx, defaultVal, col.Tp, col.Decimal) + if col.Tp != mysql.TypeTimestamp && col.Tp != mysql.TypeDatetime { + value, err := CastValue(ctx, types.NewDatum(defaultVal), col) if err != nil { - return types.Datum{}, errGetDefaultFailed.GenWithStack("Field '%s' get default value fail - %s", - col.Name, errors.Trace(err)) + return types.Datum{}, errors.Trace(err) } return value, nil } - value, err := CastValue(ctx, types.NewDatum(defaultVal), col) + // Check and get timestamp/datetime default value. + value, err := expression.GetTimeValue(ctx, defaultVal, col.Tp, col.Decimal) if err != nil { - return types.Datum{}, errors.Trace(err) + return types.Datum{}, errGetDefaultFailed.GenWithStack("Field '%s' get default value fail - %s", + col.Name, errors.Trace(err)) + } + // If column is timestamp, and default value is not current_timestamp, should convert the default value to the current session time zone. + if col.Tp == mysql.TypeTimestamp { + if vv, ok := defaultVal.(string); ok && vv != types.ZeroDatetimeStr && strings.ToUpper(vv) != strings.ToUpper(ast.CurrentTimestamp) { + t := value.GetMysqlTime() + // For col.Version = 0, the timezone information of default value is already lost, so use the system timezone as the default value timezone. + defaultTimeZone := timeutil.SystemLocation() + if col.Version == model.ColumnInfoVersion1 { + defaultTimeZone = time.UTC + } + err = t.ConvertTimeZone(defaultTimeZone, ctx.GetSessionVars().Location()) + if err != nil { + return value, errors.Trace(err) + } + value.SetMysqlTime(t) + } } return value, nil } From 694452d94fd9bf0f06f3ca89e9b9e019a55c56b0 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 21 Feb 2019 15:03:31 +0800 Subject: [PATCH 2/3] admin: fix admin check table err when add column not null, then add index on the column (#9108) --- executor/admin_test.go | 8 ++++++++ executor/executor_test.go | 2 +- util/admin/admin.go | 8 ++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/executor/admin_test.go b/executor/admin_test.go index a82096707d03e..b3da8719c2a6e 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -507,6 +507,14 @@ func (s *testSuite) TestAdminCheckTable(c *C) { tk.MustExec(`ALTER TABLE td1 ADD COLUMN c4 DECIMAL(12,8) NULL DEFAULT '213.41598062';`) tk.MustExec(`ALTER TABLE td1 ADD INDEX id2 (c4) ;`) tk.MustExec(`ADMIN CHECK TABLE td1;`) + + // Test add not null column, then add index. + tk.MustExec(`drop table if exists t1`) + tk.MustExec(`create table t1 (a int);`) + tk.MustExec(`insert into t1 set a=2;`) + tk.MustExec(`alter table t1 add column b timestamp not null;`) + tk.MustExec(`alter table t1 add index(b);`) + tk.MustExec(`admin check table t1;`) } func (s *testSuite) TestAdminCheckPrimaryIndex(c *C) { diff --git a/executor/executor_test.go b/executor/executor_test.go index f78899e1e9e2d..32af7f3d17f34 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2158,7 +2158,7 @@ func (s *testSuite) TestTimestampDefaultValueTimeZone(c *C) { tk.MustExec("set @@sql_mode='STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION';") tk.MustExec("drop table if exists t") tk.MustExec("set time_zone = '+08:00'") - tk.MustExec(`create table t (a int, b timestamp default "0000-00-00 00")`) + tk.MustExec(`create table t (a int, b timestamp default "0000-00-00 00:00:00")`) tk.MustExec("insert into t set a=1") r = tk.MustQuery(`show create table t`) r.Check(testkit.Rows("t CREATE TABLE `t` (\n" + " `a` int(11) DEFAULT NULL,\n" + " `b` timestamp DEFAULT '0000-00-00 00:00:00'\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")) diff --git a/util/admin/admin.go b/util/admin/admin.go index d8f8b0bafcb54..202a57f012e64 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -432,8 +432,8 @@ func CheckRecordAndIndex(sessCtx sessionctx.Context, txn kv.Transaction, t table for i, val := range vals1 { col := cols[i] if val.IsNull() { - if mysql.HasNotNullFlag(col.Flag) { - return false, errors.New("Miss") + if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().OriginDefaultValue == nil { + return false, errors.Errorf("Column %v define as not null, but can't find the value where handle is %v", col.Name, h1) } // NULL value is regarded as its default value. colDefVal, err := table.GetColOriginDefaultValue(sessCtx, col.ToInfo()) @@ -639,8 +639,8 @@ func rowWithCols(sessCtx sessionctx.Context, txn kv.Retriever, t table.Table, h } ri, ok := rowMap[col.ID] if !ok { - if mysql.HasNotNullFlag(col.Flag) { - return nil, errors.New("Miss") + if mysql.HasNotNullFlag(col.Flag) && col.ToInfo().OriginDefaultValue == nil { + return nil, errors.Errorf("Column %v define as not null, but can't find the value where handle is %v", col.Name, h) } // NULL value is regarded as its default value. colDefVal, err := table.GetColOriginDefaultValue(sessCtx, col.ToInfo()) From d66ddd636c5fab3db62096a147db3f371d93d8e1 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 21 Feb 2019 15:07:56 +0800 Subject: [PATCH 3/3] remove unsupported function test --- executor/write_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/executor/write_test.go b/executor/write_test.go index f9a002fe628d6..4f0aa412a4c44 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -1861,15 +1861,6 @@ func (s *testSuite) TestNullDefault(c *C) { tk.MustQuery("select * from test_null_default").Check(testkit.Rows("", "1970-01-01 08:20:34")) } -func (s *testSuite) TestNotNullDefault(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test; drop table if exists t1,t2;") - defer tk.MustExec("drop table t1,t2") - tk.MustExec("create table t1 (a int not null default null default 1);") - tk.MustExec("create table t2 (a int);") - tk.MustExec("alter table t2 change column a a int not null default null default 1;") -} - func (s *testBypassSuite) TestBypassLatch(c *C) { store, err := mockstore.NewMockTikvStore( // Small latch slot size to make conflicts.