Skip to content

Commit

Permalink
* fix timestamp default value bug in multiple time zones. (#9115) (#9467
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@crazycs520 @ngaut
crazycs520 authored and ngaut committed Feb 28, 2019
1 parent 687ecdf commit b92404e
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 32 deletions.
19 changes: 16 additions & 3 deletions ddl/column.go
View file
Expand Up @@ -247,6 +247,12 @@ func (d *ddl) 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 @@ -611,9 +617,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
}
70 changes: 50 additions & 20 deletions ddl/ddl_api.go
View file
Expand Up @@ -21,6 +21,7 @@ import (
"bytes"
"fmt"
"strings"
"time"

"github.com/cznic/mathutil"
"github.com/juju/errors"
Expand Down Expand Up @@ -268,6 +269,30 @@ func checkColumnCantHaveDefaultValue(ctx sessionctx.Context, col *table.Column,
return 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().TimeZone, 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 @@ -293,7 +318,7 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef) (
col.Flag |= mysql.NotNullFlag
}
}

var err error
setOnUpdateNow := false
hasDefaultValue := false
if colDef.Options != nil {
Expand Down Expand Up @@ -324,15 +349,10 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef) (
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.Gen("invalid default value - %s", err)
}
if err = checkColumnCantHaveDefaultValue(ctx, col, value); err != nil {
return nil, nil, errors.Trace(err)
}
col.DefaultValue = value
hasDefaultValue = true
removeOnUpdateNowFlag(col)
case ast.ColumnOptionOnUpdate:
// TODO: Support other time functions.
Expand Down Expand Up @@ -377,14 +397,15 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef) (
col.Flag &= ^mysql.BinaryFlag
col.Flag |= mysql.UnsignedFlag | mysql.ZerofillFlag
}
err := checkDefaultValue(ctx, col, hasDefaultValue)
err = checkDefaultValue(ctx, col, hasDefaultValue)
if err != nil {
return nil, nil, errors.Trace(err)
}
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 @@ -1276,13 +1297,23 @@ func modifiable(origin *types.FieldType, to *types.FieldType) error {
return errUnsupportedModifyColumn.GenByArgs(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 ErrColumnBadNull.Gen("invalid default value - %s", err)
return hasDefaultValue, ErrColumnBadNull.Gen("invalid default value - %s", err)
}

if err = checkColumnCantHaveDefaultValue(ctx, col, value); err != nil {
return hasDefaultValue, errors.Trace(err)
}
value, err = convertTimestampDefaultValToUTC(ctx, value, col)
if err != nil {
return hasDefaultValue, errors.Trace(err)
}
col.DefaultValue = value
return errors.Trace(checkDefaultValue(ctx, col, true))
hasDefaultValue = true
return hasDefaultValue, nil
}

func setColumnComment(ctx sessionctx.Context, col *table.Column, option *ast.ColumnOption) error {
Expand All @@ -1300,18 +1331,14 @@ 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.Gen("invalid default value - %s", err)
}
if err = checkColumnCantHaveDefaultValue(ctx, col, value); err != nil {
return errors.Trace(err)
}
col.DefaultValue = value
hasDefaultValue = true
case ast.ColumnOptionComment:
err := setColumnComment(ctx, col, opt)
if err != nil {
Expand Down Expand Up @@ -1528,10 +1555,13 @@ func (d *ddl) AlterColumn(ctx sessionctx.Context, ident ast.Ident, spec *ast.Alt
col.DefaultValue = nil
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
64 changes: 64 additions & 0 deletions executor/executor_test.go
View file
Expand Up @@ -31,6 +31,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"
"github.com/pingcap/tidb/model"
Expand Down Expand Up @@ -2079,6 +2080,69 @@ 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")
defer 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") CHARSET=utf8mb4 COLLATE=utf8mb4_bin`)
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") CHARSET=utf8mb4 COLLATE=utf8mb4_bin`)
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()))
}

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
View file
Expand Up @@ -269,6 +269,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 @@ -279,7 +298,7 @@ func (e *ShowExec) fetchShowColumns() error {
desc.Collation,
desc.Null,
desc.Key,
desc.DefaultValue,
columnDefault,
desc.Extra,
desc.Privileges,
desc.Comment,
Expand All @@ -290,7 +309,7 @@ func (e *ShowExec) fetchShowColumns() error {
desc.Type,
desc.Null,
desc.Key,
desc.DefaultValue,
columnDefault,
desc.Extra,
})
}
Expand Down Expand Up @@ -520,6 +539,15 @@ func (e *ShowExec) fetchShowCreateTable() error {
buf.WriteString(" DEFAULT CURRENT_TIMESTAMP")
default:
defaultValStr := fmt.Sprintf("%v", col.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
13 changes: 13 additions & 0 deletions model/model.go
View file
Expand Up @@ -58,6 +58,13 @@ func (s SchemaState) String() string {
}
}

const (
// ColumnInfoVersion0 means the column info version is 0.
ColumnInfoVersion0 = uint64(0)
// ColumnInfoVersion1 means the column info version is 1.
ColumnInfoVersion1 = uint64(1)
)

// ColumnInfo provides meta data describing of a table column.
type ColumnInfo struct {
ID int64 `json:"id"`
Expand All @@ -71,6 +78,12 @@ type ColumnInfo struct {
types.FieldType `json:"type"`
State SchemaState `json:"state"`
Comment string `json:"comment"`
// Version means the version of the column info.
// Version = 0: For OriginDefaultValue and DefaultValue of timestamp column will stores the default time in system time zone.
// That is a bug if multiple TiDB servers in different system time zone.
// 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. For compatibility with version 0, we add version field in column info struct.
Version uint64 `json:"version"`
}

// Clone clones ColumnInfo.
Expand Down
31 changes: 24 additions & 7 deletions table/column.go
View file
Expand Up @@ -19,6 +19,7 @@ package table

import (
"strings"
"time"
"unicode/utf8"

"github.com/juju/errors"
Expand Down Expand Up @@ -318,18 +319,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.Gen("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.Gen("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 := time.Local
if col.Version == model.ColumnInfoVersion1 {
defaultTimeZone = time.UTC
}
err = t.ConvertTimeZone(defaultTimeZone, ctx.GetSessionVars().GetTimeZone())
if err != nil {
return value, errors.Trace(err)
}
value.SetMysqlTime(t)
}
}
return value, nil
}
Expand Down

0 comments on commit b92404e

Please sign in to comment.