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

Merged
merged 36 commits into from
Feb 18, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
deadb33
update parser go.mod
crazycs520 Jan 17, 2019
d038251
add version field in ColumnInfo
crazycs520 Jan 17, 2019
c8a22bb
add test
crazycs520 Jan 18, 2019
b1a5d27
refine code
crazycs520 Jan 18, 2019
c97e6b8
refine code
crazycs520 Jan 18, 2019
b3305cb
fix old version timestamp multiple zone bugs.
crazycs520 Jan 18, 2019
dbbf27a
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 18, 2019
905503f
fix mysql test
crazycs520 Jan 18, 2019
5d01fee
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 18, 2019
095aa25
address comment
crazycs520 Jan 21, 2019
3a216c7
address comment
crazycs520 Jan 21, 2019
c69902a
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 21, 2019
2eac167
address comment
crazycs520 Jan 21, 2019
e891229
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 21, 2019
9f3ce16
update go.mod for parser
crazycs520 Jan 23, 2019
a8e648f
Merge branch 'master' of https://github.com/pingcap/tidb into timesta…
crazycs520 Jan 23, 2019
7cafc8d
add test for add index
crazycs520 Jan 24, 2019
e1f240f
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 28, 2019
b70ad83
address comment
crazycs520 Jan 28, 2019
f61efb6
address comment
crazycs520 Jan 28, 2019
3b9e16b
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 29, 2019
08eb3b0
address comment
crazycs520 Jan 29, 2019
f772183
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 30, 2019
2ee4d56
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Jan 30, 2019
7adec29
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Feb 11, 2019
d48ad97
Merge branch 'master' of https://github.com/pingcap/tidb into timesta…
crazycs520 Feb 12, 2019
56e5610
add comment
crazycs520 Feb 12, 2019
c332175
Merge branch 'timestamp-default-value-zone' of https://github.com/cra…
crazycs520 Feb 12, 2019
809f14e
Merge branch 'master' of https://github.com/pingcap/tidb into timesta…
crazycs520 Feb 12, 2019
930fce9
refine code
crazycs520 Feb 12, 2019
438c3dd
remove blank line
crazycs520 Feb 12, 2019
4e9cd14
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Feb 12, 2019
d5b1719
Merge branch 'master' of https://github.com/pingcap/tidb into timesta…
crazycs520 Feb 12, 2019
48645da
Merge branch 'master' into timestamp-default-value-zone
ciscoxll Feb 14, 2019
e79b821
Merge branch 'master' into timestamp-default-value-zone
zz-jason Feb 15, 2019
15b3d94
Merge branch 'master' into timestamp-default-value-zone
crazycs520 Feb 18, 2019
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
6 changes: 5 additions & 1 deletion ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,11 @@ func generateOriginDefaultValue(col *model.ColumnInfo) (interface{}, error) {

if odValue == strings.ToUpper(ast.CurrentTimestamp) &&
(col.Tp == mysql.TypeTimestamp || col.Tp == mysql.TypeDatetime) {
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
odValue = time.Now().Format(types.TimeFormat)
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
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
}
return odValue, nil
}
50 changes: 45 additions & 5 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"strings"
"sync/atomic"
"time"

"github.com/cznic/mathutil"
"github.com/pingcap/errors"
Expand Down Expand Up @@ -305,20 +306,41 @@ func checkColumnDefaultValue(ctx sessionctx.Context, col *table.Column, value in
if value != nil && ctx.GetSessionVars().SQLMode.HasNoZeroDateMode() &&
ctx.GetSessionVars().SQLMode.HasStrictMode() && types.IsTypeTime(col.Tp) {
if vv, ok := value.(string); ok {
t, err := types.ParseTime(nil, vv, col.Tp, 6)
timeValue, err := expression.GetTimeValue(ctx, vv, col.Tp, col.Decimal)
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
// Ignores ParseTime error, because ParseTime error has been dealt in getDefaultValue
// Some builtin function like CURRENT_TIMESTAMP() will cause ParseTime error.
return hasDefaultValue, value, nil
return hasDefaultValue, value, errors.Trace(err)
}
if t.Time == types.ZeroTime {
if timeValue.GetMysqlTime().Time == types.ZeroTime {
return hasDefaultValue, value, types.ErrInvalidDefault.GenWithStackByArgs(col.Name.O)
}
}
}
return hasDefaultValue, value, nil
}

func convertTimestampDelfaultValToUTC(ctx sessionctx.Context, defaultVal interface{}, col *table.Column) (interface{}, error) {
if defaultVal != nil && col.Tp == mysql.TypeTimestamp {
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
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)
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
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 Down Expand Up @@ -385,6 +407,11 @@ func columnDefToCol(ctx sessionctx.Context, offset int, colDef *ast.ColumnDef, o
if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil {
return nil, nil, errors.Trace(err)
}
value, err = convertTimestampDelfaultValToUTC(ctx, value, col)
if err != nil {
return nil, nil, errors.Trace(err)

}
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if err = col.SetDefaultValue(value); err != nil {
return nil, nil, errors.Trace(err)
}
Expand Down Expand Up @@ -1938,6 +1965,14 @@ func setDefaultValue(ctx sessionctx.Context, col *table.Column, option *ast.Colu
if err != nil {
return ErrColumnBadNull.GenWithStack("invalid default value - %s", err)
}
if _, value, err = checkColumnDefaultValue(ctx, col, value); err != nil {
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
return errors.Trace(err)
}
value, err = convertTimestampDelfaultValToUTC(ctx, value, col)
if err != nil {
return errors.Trace(err)

}
err = col.SetDefaultValue(value)
if err != nil {
return errors.Trace(err)
Expand Down Expand Up @@ -1970,6 +2005,11 @@ func setDefaultAndComment(ctx sessionctx.Context, col *table.Column, options []*
if hasDefaultValue, value, err = checkColumnDefaultValue(ctx, col, value); err != nil {
return errors.Trace(err)
}
value, err = convertTimestampDelfaultValToUTC(ctx, value, col)
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errors.Trace(err)

}
if err = col.SetDefaultValue(value); err != nil {
return errors.Trace(err)
}
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"
"github.com/pingcap/tidb/planner"
Expand Down Expand Up @@ -2188,6 +2189,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")
XuHuaiyu marked this conversation as resolved.
Show resolved Hide resolved
}

func (s *testSuite) TestTiDBCurrentTS(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustQuery("select @@tidb_current_ts").Check(testkit.Rows("0"))
Expand Down
17 changes: 17 additions & 0 deletions executor/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,14 @@ func (e *ShowExec) fetchShowColumns() error {
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())
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errors.Trace(err)
}
defaultValStr = timeValue.GetMysqlTime().String()
}
if col.Tp == mysql.TypeBit {
defaultValBinaryLiteral := types.BinaryLiteral(defaultValStr)
columnDefault = defaultValBinaryLiteral.ToBitLiteralString(true)
Expand Down Expand Up @@ -628,6 +636,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())
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errors.Trace(err)
}
defaultValStr = timeValue.GetMysqlTime().String()
}

if col.Tp == mysql.TypeBit {
defaultValBinaryLiteral := types.BinaryLiteral(defaultValStr)
fmt.Fprintf(&buf, " DEFAULT %s", defaultValBinaryLiteral.ToBitLiteralString(true))
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ require (
github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3
github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e
github.com/pingcap/parser v0.0.0-20190123063514-f8c3dff115d5
github.com/pingcap/pd v2.1.0-rc.4+incompatible
github.com/pingcap/tidb-tools v2.1.3-0.20190116051332-34c808eef588+incompatible
github.com/pingcap/tipb v0.0.0-20181012112600-11e33c750323
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e h1:P73/4dPCL96rG
github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw=
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379 h1:l4KInBOtxjbgQLjCFHzX66vZgNzsH4a+RiuVZGrO0xk=
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379/go.mod h1:QMdbTAXCHzzygQzqcG9uVUgU2fKeSN1GmfMiykdSzzY=
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e h1:UxQ0Fj4NDpQ0SSZ0Z76je8OPSDlbRxMfXTbyPrFs6c4=
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/parser v0.0.0-20190123063514-f8c3dff115d5 h1:WBU3Axs+DW+2GxtyDsyNUSd5Sp0C0SVAZ5GNuDsfhGU=
github.com/pingcap/parser v0.0.0-20190123063514-f8c3dff115d5/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/pd v2.1.0-rc.4+incompatible h1:/buwGk04aHO5odk/+O8ZOXGs4qkUjYTJ2UpCJXna8NE=
github.com/pingcap/pd v2.1.0-rc.4+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E=
github.com/pingcap/tidb-tools v2.1.3-0.20190116051332-34c808eef588+incompatible h1:e9Gi/LP9181HT3gBfSOeSBA+5JfemuE4aEAhqNgoE4k=
Expand Down
20 changes: 20 additions & 0 deletions table/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package table

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

"github.com/pingcap/errors"
Expand All @@ -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"
)

Expand Down Expand Up @@ -357,6 +359,24 @@ func getColDefaultValue(ctx sessionctx.Context, col *model.ColumnInfo, defaultVa
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 {
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
if vv, ok := defaultVal.(string); ok {
if 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()
crazycs520 marked this conversation as resolved.
Show resolved Hide resolved
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
}
value, err := CastValue(ctx, types.NewDatum(defaultVal), col)
Expand Down