From 7f6d43a3472cec984824c5fdca72d3fe1bebb3a2 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Thu, 4 Apr 2019 21:50:52 +0800 Subject: [PATCH 1/5] expression: check timezone when encoding timestamp datum --- expression/distsql_builtin.go | 2 +- expression/expr_to_pb.go | 13 +++++++++++-- expression/integration_test.go | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/expression/distsql_builtin.go b/expression/distsql_builtin.go index 96ac0397d0cd5..127e6baa95c78 100644 --- a/expression/distsql_builtin.go +++ b/expression/distsql_builtin.go @@ -557,7 +557,7 @@ func convertTime(data []byte, ftPB *tipb.FieldType, tz *time.Location) (*Constan if err != nil { return nil, err } - if ft.Tp == mysql.TypeTimestamp && !t.IsZero() { + if ft.Tp == mysql.TypeTimestamp && tz != time.UTC { err = t.ConvertTimeZone(time.UTC, tz) if err != nil { return nil, err diff --git a/expression/expr_to_pb.go b/expression/expr_to_pb.go index 2caecae8cb059..914a8d63af205 100644 --- a/expression/expr_to_pb.go +++ b/expression/expr_to_pb.go @@ -15,6 +15,7 @@ package expression import ( "context" + "time" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/charset" @@ -106,7 +107,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { logutil.Logger(context.Background()).Error("eval constant or correlated column", zap.String("expression", expr.ExplainInfo()), zap.Error(err)) return nil } - tp, val, ok := pc.encodeDatum(d) + tp, val, ok := pc.encodeDatum(d, ft) if !ok { return nil } @@ -117,7 +118,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { return &tipb.Expr{Tp: tp, Val: val, FieldType: ToPBFieldType(ft)} } -func (pc *PbConverter) encodeDatum(d types.Datum) (tipb.ExprType, []byte, bool) { +func (pc *PbConverter) encodeDatum(d types.Datum, ft *types.FieldType) (tipb.ExprType, []byte, bool) { var ( tp tipb.ExprType val []byte @@ -158,6 +159,14 @@ func (pc *PbConverter) encodeDatum(d types.Datum) (tipb.ExprType, []byte, bool) if pc.client.IsRequestTypeSupported(kv.ReqTypeDAG, int64(tipb.ExprType_MysqlTime)) { tp = tipb.ExprType_MysqlTime t := d.GetMysqlTime() + // To be compatible with `encode` and `PBToExpr`, convert timestamp to UTC timezone. + if ft.Tp == mysql.TypeTimestamp && pc.sc.TimeZone != time.UTC { + err := t.ConvertTimeZone(pc.sc.TimeZone, time.UTC) + if err != nil { + logutil.Logger(context.Background()).Error("encode mysql time", zap.Error(err)) + return tp, nil, false + } + } v, err := t.ToPackedUint() if err != nil { logutil.Logger(context.Background()).Error("encode mysql time", zap.Error(err)) diff --git a/expression/integration_test.go b/expression/integration_test.go index 33b8f5a8076da..e67e0b84964dc 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -4235,3 +4235,17 @@ where datediff(b.date8, date(from_unixtime(a.starttime))) >= 0` tk.MustQuery(q) } + +func (s *testIntegrationSuite) TestTimestampDatumEncode(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec(`drop table if exists t;`) + tk.MustExec(`create table t (a bigint primary key, b timestamp)`) + tk.MustExec(`insert into t values (1, "2019-04-29 11:56:12")`) + tk.MustQuery(`explain select * from t where b = (select max(b) from t)`).Check(testkit.Rows( + "TableReader_43 10.00 root data:Selection_42", + "└─Selection_42 10.00 cop eq(test.t.b, 2019-04-29 11:56:12)", + " └─TableScan_41 10000.00 cop table:t, range:[-inf,+inf], keep order:false, stats:pseudo", + )) + tk.MustQuery(`select * from t where b = (select max(b) from t)`).Check(testkit.Rows(`1 2019-04-29 11:56:12`)) +} From b184ce05059099b469308636876e6f570bcf9105 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Mon, 29 Apr 2019 15:45:14 +0800 Subject: [PATCH 2/5] address comments --- expression/expr_to_pb.go | 13 +------------ util/codec/codec.go | 35 ++++++++++++++++++++++------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/expression/expr_to_pb.go b/expression/expr_to_pb.go index 914a8d63af205..97289fd437376 100644 --- a/expression/expr_to_pb.go +++ b/expression/expr_to_pb.go @@ -15,7 +15,6 @@ package expression import ( "context" - "time" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/charset" @@ -158,21 +157,11 @@ func (pc *PbConverter) encodeDatum(d types.Datum, ft *types.FieldType) (tipb.Exp case types.KindMysqlTime: if pc.client.IsRequestTypeSupported(kv.ReqTypeDAG, int64(tipb.ExprType_MysqlTime)) { tp = tipb.ExprType_MysqlTime - t := d.GetMysqlTime() - // To be compatible with `encode` and `PBToExpr`, convert timestamp to UTC timezone. - if ft.Tp == mysql.TypeTimestamp && pc.sc.TimeZone != time.UTC { - err := t.ConvertTimeZone(pc.sc.TimeZone, time.UTC) - if err != nil { - logutil.Logger(context.Background()).Error("encode mysql time", zap.Error(err)) - return tp, nil, false - } - } - v, err := t.ToPackedUint() + val, err := codec.EncodeMySQLTime(pc.sc, d, nil) if err != nil { logutil.Logger(context.Background()).Error("encode mysql time", zap.Error(err)) return tp, nil, false } - val = codec.EncodeUint(nil, v) return tp, val, true } return tp, nil, false diff --git a/util/codec/codec.go b/util/codec/codec.go index fde8afe1b7bd3..b2d5a988ea8be 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -67,21 +67,10 @@ func encode(sc *stmtctx.StatementContext, b []byte, vals []types.Datum, comparab b = encodeBytes(b, vals[i].GetBytes(), comparable) case types.KindMysqlTime: b = append(b, uintFlag) - t := vals[i].GetMysqlTime() - // Encoding timestamp need to consider timezone. - // If it's not in UTC, transform to UTC first. - if t.Type == mysql.TypeTimestamp && sc.TimeZone != time.UTC { - err = t.ConvertTimeZone(sc.TimeZone, time.UTC) - if err != nil { - return nil, errors.Trace(err) - } - } - var v uint64 - v, err = t.ToPackedUint() + b, err = EncodeMySQLTime(sc, vals[i], b) if err != nil { - return nil, errors.Trace(err) + return nil, err } - b = EncodeUint(b, v) case types.KindMysqlDuration: // duration may have negative value, so we cannot use String to encode directly. b = append(b, durationFlag) @@ -134,6 +123,26 @@ func encode(sc *stmtctx.StatementContext, b []byte, vals []types.Datum, comparab return b, errors.Trace(err) } +// EncodeMySQLTime encodes datum of `KindMysqlTime` to []byte. +func EncodeMySQLTime(sc *stmtctx.StatementContext, d types.Datum, b []byte) (_ []byte, err error) { + t := d.GetMysqlTime() + // Encoding timestamp need to consider timezone. If it's not in UTC, transform to UTC first. + // This is compatible with `PBToExpr > convertTime`, and coprocessor assumes the passed timestamp is in UTC as well. + if t.Type == mysql.TypeTimestamp && sc.TimeZone != time.UTC { + err = t.ConvertTimeZone(sc.TimeZone, time.UTC) + if err != nil { + return nil, err + } + } + var v uint64 + v, err = t.ToPackedUint() + if err != nil { + return nil, err + } + b = EncodeUint(b, v) + return b, nil +} + func encodeBytes(b []byte, v []byte, comparable bool) []byte { if comparable { b = append(b, bytesFlag) From 6c42bced2e91016dcfd34abc4528f61991ed89b5 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Mon, 29 Apr 2019 15:54:49 +0800 Subject: [PATCH 3/5] tiny fix --- expression/expr_to_pb.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expression/expr_to_pb.go b/expression/expr_to_pb.go index 97289fd437376..a9837248dfb68 100644 --- a/expression/expr_to_pb.go +++ b/expression/expr_to_pb.go @@ -106,7 +106,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { logutil.Logger(context.Background()).Error("eval constant or correlated column", zap.String("expression", expr.ExplainInfo()), zap.Error(err)) return nil } - tp, val, ok := pc.encodeDatum(d, ft) + tp, val, ok := pc.encodeDatum(d) if !ok { return nil } @@ -117,7 +117,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { return &tipb.Expr{Tp: tp, Val: val, FieldType: ToPBFieldType(ft)} } -func (pc *PbConverter) encodeDatum(d types.Datum, ft *types.FieldType) (tipb.ExprType, []byte, bool) { +func (pc *PbConverter) encodeDatum(d types.Datum) (tipb.ExprType, []byte, bool) { var ( tp tipb.ExprType val []byte From 86ea0813f98bf9334cba8dac0f6ae7737dd8be4f Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Mon, 29 Apr 2019 19:48:41 +0800 Subject: [PATCH 4/5] use correct type --- expression/expr_to_pb.go | 6 +++--- session/session.go | 7 +++++++ util/codec/codec.go | 9 ++++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/expression/expr_to_pb.go b/expression/expr_to_pb.go index a9837248dfb68..4416b464c1f4c 100644 --- a/expression/expr_to_pb.go +++ b/expression/expr_to_pb.go @@ -106,7 +106,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { logutil.Logger(context.Background()).Error("eval constant or correlated column", zap.String("expression", expr.ExplainInfo()), zap.Error(err)) return nil } - tp, val, ok := pc.encodeDatum(d) + tp, val, ok := pc.encodeDatum(ft, d) if !ok { return nil } @@ -117,7 +117,7 @@ func (pc PbConverter) conOrCorColToPBExpr(expr Expression) *tipb.Expr { return &tipb.Expr{Tp: tp, Val: val, FieldType: ToPBFieldType(ft)} } -func (pc *PbConverter) encodeDatum(d types.Datum) (tipb.ExprType, []byte, bool) { +func (pc *PbConverter) encodeDatum(ft *types.FieldType, d types.Datum) (tipb.ExprType, []byte, bool) { var ( tp tipb.ExprType val []byte @@ -157,7 +157,7 @@ func (pc *PbConverter) encodeDatum(d types.Datum) (tipb.ExprType, []byte, bool) case types.KindMysqlTime: if pc.client.IsRequestTypeSupported(kv.ReqTypeDAG, int64(tipb.ExprType_MysqlTime)) { tp = tipb.ExprType_MysqlTime - val, err := codec.EncodeMySQLTime(pc.sc, d, nil) + val, err := codec.EncodeMySQLTime(pc.sc, d, ft.Tp, nil) if err != nil { logutil.Logger(context.Background()).Error("encode mysql time", zap.Error(err)) return tp, nil, false diff --git a/session/session.go b/session/session.go index 7c57368857127..ee10adc76af95 100644 --- a/session/session.go +++ b/session/session.go @@ -971,6 +971,13 @@ func (s *session) execute(ctx context.Context, sql string) (recordSets []sqlexec if err != nil { return nil, err } + flag := true + if err == nil { + flag = false + } + if flag { + return nil, errors.New("eurekaka") + } charsetInfo, collation := s.sessionVars.GetCharsetInfo() diff --git a/util/codec/codec.go b/util/codec/codec.go index b2d5a988ea8be..113dabf780ced 100644 --- a/util/codec/codec.go +++ b/util/codec/codec.go @@ -67,7 +67,7 @@ func encode(sc *stmtctx.StatementContext, b []byte, vals []types.Datum, comparab b = encodeBytes(b, vals[i].GetBytes(), comparable) case types.KindMysqlTime: b = append(b, uintFlag) - b, err = EncodeMySQLTime(sc, vals[i], b) + b, err = EncodeMySQLTime(sc, vals[i], mysql.TypeUnspecified, b) if err != nil { return nil, err } @@ -124,11 +124,14 @@ func encode(sc *stmtctx.StatementContext, b []byte, vals []types.Datum, comparab } // EncodeMySQLTime encodes datum of `KindMysqlTime` to []byte. -func EncodeMySQLTime(sc *stmtctx.StatementContext, d types.Datum, b []byte) (_ []byte, err error) { +func EncodeMySQLTime(sc *stmtctx.StatementContext, d types.Datum, tp byte, b []byte) (_ []byte, err error) { t := d.GetMysqlTime() // Encoding timestamp need to consider timezone. If it's not in UTC, transform to UTC first. // This is compatible with `PBToExpr > convertTime`, and coprocessor assumes the passed timestamp is in UTC as well. - if t.Type == mysql.TypeTimestamp && sc.TimeZone != time.UTC { + if tp == mysql.TypeUnspecified { + tp = t.Type + } + if tp == mysql.TypeTimestamp && sc.TimeZone != time.UTC { err = t.ConvertTimeZone(sc.TimeZone, time.UTC) if err != nil { return nil, err From b4fe1c4b7e5241628a9c3c0d6318ffb1a7ff8381 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Mon, 29 Apr 2019 19:55:09 +0800 Subject: [PATCH 5/5] remove code --- session/session.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/session/session.go b/session/session.go index dacfe5268ca3c..1f504b9dd4ddd 100644 --- a/session/session.go +++ b/session/session.go @@ -971,13 +971,6 @@ func (s *session) execute(ctx context.Context, sql string) (recordSets []sqlexec if err != nil { return nil, err } - flag := true - if err == nil { - flag = false - } - if flag { - return nil, errors.New("eurekaka") - } charsetInfo, collation := s.sessionVars.GetCharsetInfo()