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

Jump to bottom

expression,table: fix insert partitioned table bug when the time zone change #14370

Merged
merged 6 commits into from
Jan 14, 2020
Merged

expression,table: fix insert partitioned table bug when the time zone change #14370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9bba590
expression,table: fix insert partitioned table bug when the time zone…
tiancaiamao Jan 7, 2020
003bc78
Merge branch 'master' into partition-timezone-change
tiancaiamao Jan 7, 2020
58563fe
Update table/tables/partition_test.go
tiancaiamao Jan 7, 2020
19dc8a0
address comment
tiancaiamao Jan 7, 2020
5e0b4ce
Merge branch 'master' into partition-timezone-change
bb7133 Jan 11, 2020
3b0e34f
Merge branch 'master' into partition-timezone-change
tiancaiamao Jan 14, 2020
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
4 changes: 4 additions & 0 deletions expression/builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,10 @@ type builtinFunc interface {
Clone() builtinFunc
}

type builtinFuncNew interface {
evalIntWithCtx(ctx sessionctx.Context, row chunk.Row) (val int64, isNull bool, err error)
}

// baseFunctionClass will be contained in every struct that implement functionClass interface.
type baseFunctionClass struct {
funcName string
Expand Down
4 changes: 4 additions & 0 deletions expression/builtin_compare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,10 @@ func (b *builtinLTIntSig) Clone() builtinFunc {
return newSig
}

func (b *builtinLTIntSig) evalIntWithCtx(ctx sessionctx.Context, row chunk.Row) (val int64, isNull bool, err error) {
return resOfLT(CompareInt(ctx, b.args[0], b.args[1], row, row))
}

func (b *builtinLTIntSig) evalInt(row chunk.Row) (val int64, isNull bool, err error) {
return resOfLT(CompareInt(b.ctx, b.args[0], b.args[1], row, row))
}
Expand Down
14 changes: 12 additions & 2 deletions expression/builtin_time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ var (
_ functionClass = &subDateFunctionClass{}
)

var (
_ builtinFuncNew = &builtinUnixTimestampIntSig{}
)

var (
_ builtinFunc = &builtinDateSig{}
_ builtinFunc = &builtinDateLiteralSig{}
Expand Down Expand Up @@ -4657,15 +4661,21 @@ func (b *builtinUnixTimestampIntSig) Clone() builtinFunc {
// evalInt evals a UNIX_TIMESTAMP(time).
// See https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_unix-timestamp
func (b *builtinUnixTimestampIntSig) evalInt(row chunk.Row) (int64, bool, error) {
val, isNull, err := b.args[0].EvalTime(b.ctx, row)
return b.evalIntWithCtx(b.ctx, row)
}

func (b *builtinUnixTimestampIntSig) evalIntWithCtx(ctx sessionctx.Context, row chunk.Row) (int64, bool, error) {
val, isNull, err := b.args[0].EvalTime(ctx, row)
if err != nil && terror.ErrorEqual(types.ErrWrongValue.GenWithStackByArgs(types.TimeStr, val), err) {
// Return 0 for invalid date time.
return 0, false, nil
}
if isNull {
return 0, true, nil
}
t, err := val.GoTime(getTimeZone(b.ctx))

tz := ctx.GetSessionVars().Location()
t, err := val.GoTime(tz)
if err != nil {
return 0, false, nil
}
Expand Down
3 changes: 3 additions & 0 deletions expression/scalar_function.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,9 @@ func (sf *ScalarFunction) Eval(row chunk.Row) (d types.Datum, err error) {

// EvalInt implements Expression interface.
func (sf *ScalarFunction) EvalInt(ctx sessionctx.Context, row chunk.Row) (int64, bool, error) {
if f, ok := sf.Function.(builtinFuncNew); ok {
return f.evalIntWithCtx(ctx, row)
}
return sf.Function.evalInt(row)
Copy link
Contributor

Choose a reason for hiding this comment

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

For using the context in current session. I think of another way:
How about add a new function setCtx for struct builtinFunc,
and change here as:

Suggested change
return sf.Function.evalInt(row)
sf.Function.setCtx(ctx)
return sf.Function.evalInt(row)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The setCtx way is ugly IMHO. @Deardrops
The real problem is, the session context is not something belongs to a function.
As long as function store the ctx, the function is invalid when the session context changes.
So the right way to fix the problem is removing ctx from the struct, instead of providing setCtx as a workaround.

}

Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs=
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f h1:J5lckAjkw6qYlOZNj90mLYNTEKDvWeuc1yieZ8qUzUE=
golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f/go.mod h1:5qLYkcX4OjUUV8bRuDixDT3tpyyb+LUpUlRWLxfhWrs=
Expand Down
57 changes: 57 additions & 0 deletions table/tables/partition_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,63 @@ func (ts *testSuite) TestLocateRangePartitionErr(c *C) {
c.Assert(table.ErrNoPartitionForGivenValue.Equal(err), IsTrue)
}

func (ts *testSuite) TestTimeZoneChange(c *C) {
tk := testkit.NewTestKitWithInit(c, ts.store)
tk.MustExec("use test")
createTable := `CREATE TABLE timezone_test (
tiancaiamao marked this conversation as resolved.
Show resolved Hide resolved
id int(11) NOT NULL,
creation_dt timestamp DEFAULT CURRENT_TIMESTAMP ) PARTITION BY RANGE ( unix_timestamp(creation_dt) )
( PARTITION p5 VALUES LESS THAN ( UNIX_TIMESTAMP('2020-01-03 15:10:00') ),
PARTITION p6 VALUES LESS THAN ( UNIX_TIMESTAMP('2020-01-03 15:15:00') ),
PARTITION p7 VALUES LESS THAN ( UNIX_TIMESTAMP('2020-01-03 15:20:00') ),
PARTITION p8 VALUES LESS THAN ( UNIX_TIMESTAMP('2020-01-03 15:25:00') ),
PARTITION p9 VALUES LESS THAN (MAXVALUE) )`
tk.MustExec("SET @@time_zone = 'Asia/Shanghai'")
tk.MustExec(createTable)
tk.MustQuery("SHOW CREATE TABLE timezone_test").Check(testkit.Rows("timezone_test CREATE TABLE `timezone_test` (\n" +
" `id` int(11) NOT NULL,\n" +
" `creation_dt` timestamp DEFAULT CURRENT_TIMESTAMP\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
"PARTITION BY RANGE ( unix_timestamp(`creation_dt`) ) (\n" +
" PARTITION p5 VALUES LESS THAN (1578035400),\n" +
" PARTITION p6 VALUES LESS THAN (1578035700),\n" +
" PARTITION p7 VALUES LESS THAN (1578036000),\n" +
" PARTITION p8 VALUES LESS THAN (1578036300),\n" +
" PARTITION p9 VALUES LESS THAN (MAXVALUE)\n)"))
tk.MustExec("DROP TABLE timezone_test")

// Note that the result of "show create table" varies with time_zone.
Deardrops marked this conversation as resolved.
Show resolved Hide resolved
tk.MustExec("SET @@time_zone = 'UTC'")
tk.MustExec(createTable)
tk.MustQuery("SHOW CREATE TABLE timezone_test").Check(testkit.Rows("timezone_test CREATE TABLE `timezone_test` (\n" +
" `id` int(11) NOT NULL,\n" +
" `creation_dt` timestamp DEFAULT CURRENT_TIMESTAMP\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
"PARTITION BY RANGE ( unix_timestamp(`creation_dt`) ) (\n" +
" PARTITION p5 VALUES LESS THAN (1578064200),\n" +
" PARTITION p6 VALUES LESS THAN (1578064500),\n" +
" PARTITION p7 VALUES LESS THAN (1578064800),\n" +
" PARTITION p8 VALUES LESS THAN (1578065100),\n" +
" PARTITION p9 VALUES LESS THAN (MAXVALUE)\n)"))

// Change time zone and insert data, check the data locates in the correct partition.
tk.MustExec("SET @@time_zone = 'Asia/Shanghai'")
tk.MustExec("INSERT INTO timezone_test VALUES (1,'2020-01-03 15:16:59')")
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p5)").Check(testkit.Rows("1 2020-01-03 15:16:59"))
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p6)").Check(testkit.Rows())
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p7)").Check(testkit.Rows())
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p8)").Check(testkit.Rows())
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p9)").Check(testkit.Rows())

tk.MustExec("SET @@time_zone = 'UTC'")
tk.MustExec("INSERT INTO timezone_test VALUES (1,'2020-01-03 15:16:59')")
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p5)").Check(testkit.Rows("1 2020-01-03 07:16:59"))
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p6)").Check(testkit.Rows())
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p7)").Check(testkit.Rows("1 2020-01-03 15:16:59"))
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p8)").Check(testkit.Rows())
tk.MustQuery("SELECT * FROM timezone_test PARTITION (p9)").Check(testkit.Rows())
}

func (ts *testSuite) TestCreatePartitionTableNotSupport(c *C) {
tk := testkit.NewTestKitWithInit(c, ts.store)
tk.MustExec("use test")
Expand Down