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.

Sign up for GitHub

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

expression: fix issue that results of unix_timestamp()-unix_timestamp(now()) is wrong and not stable #9884

Merged
merged 10 commits into from
Mar 29, 2019
Merged

expression: fix issue that results of unix_timestamp()-unix_timestamp(now()) is wrong and not stable #9884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e82f2b9
truncate time instead of round it
qw4990 Mar 25, 2019
f73a414
refmt
qw4990 Mar 25, 2019
2b4a0cb
add useTrunc parameter into convertTimeToMysqlTime
qw4990 Mar 25, 2019
a15f673
add UT
qw4990 Mar 25, 2019
0058d8e
update unix_timestamp
qw4990 Mar 25, 2019
b73ede3
fix CI
qw4990 Mar 27, 2019
c7f8ed4
update UT
qw4990 Mar 27, 2019
4149a28
update
qw4990 Mar 28, 2019
b1332d7
update comments
qw4990 Mar 28, 2019
364ca0c
Merge branch 'master' into fix_9710
XuHuaiyu Mar 29, 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
2 changes: 1 addition & 1 deletion executor/executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3371,7 +3371,7 @@ func (s *testSuite3) TestCurrentTimestampValueSelection(c *C) {
tk.MustQuery("select id from t where t0 = ?", t0).Check(testkit.Rows("1"))
tk.MustQuery("select id from t where t1 = ?", t1).Check(testkit.Rows("1"))
tk.MustQuery("select id from t where t2 = ?", t2).Check(testkit.Rows("1"))
time.Sleep(time.Second / 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Contributor Author

@qw4990 qw4990 Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two calls of now() around this sleep().
One is used to insert a record t0=now() and the other is used to update it set t0=now().
Before this PR, since now() rounds the result, so we can assume that these two calls of now() must get two time with different seconds and the update operation really happens.
After this PR, this assumption is broken, so I increase the waiting time to ensure they can get two time with different seconds.

time.Sleep(time.Second)
tk.MustExec("update t set t0 = now() where id = 1")
rs = tk.MustQuery("select t2 from t where id = 1")
newT2 := rs.Rows()[0][0].(string)
Expand Down
39 changes: 31 additions & 8 deletions expression/builtin_time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,14 @@ var (
_ builtinFunc = &builtinSubDateDatetimeDecimalSig{}
)

func convertTimeToMysqlTime(t time.Time, fsp int) (types.Time, error) {
tr, err := types.RoundFrac(t, fsp)
func convertTimeToMysqlTime(t time.Time, fsp int, roundMode types.RoundMode) (types.Time, error) {
var tr time.Time
var err error
if roundMode == types.ModeTruncate {
tr, err = types.TruncateFrac(t, fsp)
} else {
tr, err = types.RoundFrac(t, fsp)
}
if err != nil {
return types.Time{}, err
}
Expand Down Expand Up @@ -1605,7 +1611,7 @@ func evalFromUnixTime(ctx sessionctx.Context, fsp int, row chunk.Row, arg Expres

sc := ctx.GetSessionVars().StmtCtx
tmp := time.Unix(integralPart, fractionalPart).In(sc.TimeZone)
t, err := convertTimeToMysqlTime(tmp, fsp)
t, err := convertTimeToMysqlTime(tmp, fsp, types.ModeHalfEven)
if err != nil {
return res, true, err
}
Expand Down Expand Up @@ -1927,7 +1933,7 @@ func (b *builtinSysDateWithFspSig) evalTime(row chunk.Row) (d types.Time, isNull

loc := b.ctx.GetSessionVars().Location()
now := time.Now().In(loc)
result, err := convertTimeToMysqlTime(now, int(fsp))
result, err := convertTimeToMysqlTime(now, int(fsp), types.ModeHalfEven)
if err != nil {
return types.Time{}, true, err
}
Expand All @@ -1949,7 +1955,7 @@ func (b *builtinSysDateWithoutFspSig) Clone() builtinFunc {
func (b *builtinSysDateWithoutFspSig) evalTime(row chunk.Row) (d types.Time, isNull bool, err error) {
tz := b.ctx.GetSessionVars().Location()
now := time.Now().In(tz)
result, err := convertTimeToMysqlTime(now, 0)
result, err := convertTimeToMysqlTime(now, 0, types.ModeHalfEven)
if err != nil {
return types.Time{}, true, err
}
Expand Down Expand Up @@ -2278,7 +2284,7 @@ func evalUTCTimestampWithFsp(ctx sessionctx.Context, fsp int) (types.Time, bool,
if nowTs.Equal(time.Time{}) {
*nowTs = time.Now()
}
result, err := convertTimeToMysqlTime(nowTs.UTC(), fsp)
result, err := convertTimeToMysqlTime(nowTs.UTC(), fsp, types.ModeHalfEven)
if err != nil {
return types.Time{}, true, err
}
Expand Down Expand Up @@ -2370,7 +2376,15 @@ func evalNowWithFsp(ctx sessionctx.Context, fsp int) (types.Time, bool, error) {
}
}

result, err := convertTimeToMysqlTime(*sysTs, fsp)
// In MySQL's implementation, now() will truncate the result instead of rounding it.
// Results below are from MySQL 5.7, which can prove it.
// mysql> select now(6), now(3), now();
// +----------------------------+-------------------------+---------------------+
// | now(6) | now(3) | now() |
// +----------------------------+-------------------------+---------------------+
// | 2019-03-25 15:57:56.612966 | 2019-03-25 15:57:56.612 | 2019-03-25 15:57:56 |
// +----------------------------+-------------------------+---------------------+
result, err := convertTimeToMysqlTime(*sysTs, fsp, types.ModeTruncate)
if err != nil {
return types.Time{}, true, err
}
Expand Down Expand Up @@ -3577,7 +3591,16 @@ func goTimeToMysqlUnixTimestamp(t time.Time, decimal int) (*types.MyDecimal, err
if err != nil {
return nil, err
}
err = dec.Round(dec, decimal, types.ModeHalfEven)

// In MySQL's implementation, unix_timestamp() will truncate the result instead of rounding it.
// Results below are from MySQL 5.7, which can prove it.
// mysql> select unix_timestamp(), unix_timestamp(now(0)), now(0), unix_timestamp(now(3)), now(3), now(6);
// +------------------+------------------------+---------------------+------------------------+-------------------------+----------------------------+
// | unix_timestamp() | unix_timestamp(now(0)) | now(0) | unix_timestamp(now(3)) | now(3) | now(6) |
// +------------------+------------------------+---------------------+------------------------+-------------------------+----------------------------+
// | 1553503194 | 1553503194 | 2019-03-25 16:39:54 | 1553503194.992 | 2019-03-25 16:39:54.992 | 2019-03-25 16:39:54.992969 |
// +------------------+------------------------+---------------------+------------------------+-------------------------+----------------------------+
err = dec.Round(dec, decimal, types.ModeTruncate)
return dec, err
}

Expand Down
30 changes: 30 additions & 0 deletions expression/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"sort"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -4052,3 +4053,32 @@ func (s *testIntegrationSuite) TestIssue9325(c *C) {
result = tk.MustQuery("select * from t where a < timestamp'2019-02-16 14:21:00'")
result.Check(testkit.Rows("2019-02-16 14:19:59", "2019-02-16 14:20:01"))
}

func (s *testIntegrationSuite) TestIssue9710(c *C) {
tk := testkit.NewTestKit(c, s.store)
getSAndMS := func(str string) (int, int) {
results := strings.Split(str, ":")
SAndMS := strings.Split(results[len(results)-1], ".")
var s, ms int
s, _ = strconv.Atoi(SAndMS[0])
if len(SAndMS) > 1 {
ms, _ = strconv.Atoi(SAndMS[1])
}
return s, ms
}

for {
rs := tk.MustQuery("select now(), now(6), unix_timestamp(), unix_timestamp(now())")
s, ms := getSAndMS(rs.Rows()[0][1].(string))
if ms < 500000 {
time.Sleep(time.Second / 10)
continue
}

s1, _ := getSAndMS(rs.Rows()[0][0].(string))
c.Assert(s, Equals, s1) // now() will truncate the result instead of rounding it

c.Assert(rs.Rows()[0][2], Equals, rs.Rows()[0][3]) // unix_timestamp() will truncate the result
break
}
}
10 changes: 10 additions & 0 deletions types/time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,16 @@ func RoundFrac(t gotime.Time, fsp int) (gotime.Time, error) {
return t.Round(gotime.Duration(math.Pow10(9-fsp)) * gotime.Nanosecond), nil
}

// TruncateFrac truncates fractional seconds precision with new fsp and returns a new one.
// 2011:11:11 10:10:10.888888 round 0 -> 2011:11:11 10:10:10
// 2011:11:11 10:10:10.111111 round 0 -> 2011:11:11 10:10:10
func TruncateFrac(t gotime.Time, fsp int) (gotime.Time, error) {
if _, err := CheckFsp(fsp); err != nil {
return t, err
}
return t.Truncate(gotime.Duration(math.Pow10(9-fsp)) * gotime.Nanosecond), nil
}

// ToPackedUint encodes Time to a packed uint64 value.
//
// 1 bit 0
Expand Down