Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

* fix timestamp default value bug in multiple time zones. (#9115) (#9108) #9399

Merged
merged 3 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
73 changes: 50 additions & 23 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"bytes"
"fmt"
"strings"
"time"

"github.com/cznic/mathutil"
"github.com/pingcap/errors"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand All @@ -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:
Expand Down Expand Up @@ -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{
Expand Down
8 changes: 8 additions & 0 deletions executor/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
69 changes: 69 additions & 0 deletions executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: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"))
Expand Down
32 changes: 30 additions & 2 deletions executor/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -286,7 +305,7 @@ func (e *ShowExec) fetchShowColumns() error {
desc.Collation,
desc.Null,
desc.Key,
desc.DefaultValue,
columnDefault,
desc.Extra,
desc.Privileges,
desc.Comment,
Expand All @@ -297,7 +316,7 @@ func (e *ShowExec) fetchShowColumns() error {
desc.Type,
desc.Null,
desc.Key,
desc.DefaultValue,
columnDefault,
desc.Extra,
})
}
Expand Down Expand Up @@ -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)))
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
Loading