From 06810e589fc41a5144030f1505201ce34bdbbd76 Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 20 Jan 2025 21:37:49 +0100 Subject: [PATCH] Clean up duplicate datetime parsing code This cleans up some old legacy code which predates our much improved parsing capability with the `datetime` package. Let's use that and remove the now unused code from `sqltypes`. The conversion is only used in the driver, so move it there then and use `datetime` directly. Signed-off-by: Dirkjan Bussink --- go/sqltypes/value.go | 95 ----------------------- go/sqltypes/value_test.go | 140 ---------------------------------- go/vt/vitessdriver/convert.go | 58 +++++++++++++- 3 files changed, 56 insertions(+), 237 deletions(-) diff --git a/go/sqltypes/value.go b/go/sqltypes/value.go index 7fb6fa80396..438b51a13ba 100644 --- a/go/sqltypes/value.go +++ b/go/sqltypes/value.go @@ -26,7 +26,6 @@ import ( "math/big" "strconv" "strings" - "time" "google.golang.org/protobuf/encoding/protowire" @@ -436,100 +435,6 @@ func (v Value) String() string { return fmt.Sprintf("%v(%s)", Type(v.typ), v.val) } -// ToTime returns the value as a time.Time in UTC. -// NULL values are returned as zero time. -func (v Value) ToTime() (time.Time, error) { - return v.ToTimeInLocation(time.UTC) -} - -// ToTimeInLocation returns the value as a time.Time in the provided location. -// NULL values are returned as zero time. -func (v Value) ToTimeInLocation(loc *time.Location) (time.Time, error) { - if v.Type() == Null { - return time.Time{}, nil - } - switch v.Type() { - case Datetime, Timestamp: - return datetimeToNative(v, loc) - case Date: - return dateToNative(v, loc) - default: - return time.Time{}, ErrIncompatibleTypeCast - } -} - -// ErrInvalidTime is returned when we fail to parse a datetime -// string from MySQL. This should never happen unless things are -// seriously messed up. -var ErrInvalidTime = errors.New("invalid MySQL time string") - -var isoTimeFormat = "2006-01-02 15:04:05.999999" -var isoNullTime = "0000-00-00 00:00:00.000000" -var isoTimeLength = len(isoTimeFormat) - -// parseISOTime pases a time string in MySQL's textual datetime format. -// This is very similar to ISO8601, with some differences: -// -// - There is no T separator between the date and time sections; -// a space is used instead. -// - There is never a timezone section in the string, as these datetimes -// are not timezone-aware. There isn't a Z value for UTC times for -// the same reason. -// -// Note that this function can handle both DATE (which should _always_ have -// a length of 10) and DATETIME strings (which have a variable length, 18+ -// depending on the number of decimal sub-second places). -// -// Also note that this function handles the case where MySQL returns a NULL -// time (with a string where all sections are zeroes) by returning a zeroed -// out time.Time object. NULL time strings are not considered a parsing error. -// -// See: isoTimeFormat -func parseISOTime(tstr string, loc *time.Location, minLen, maxLen int) (t time.Time, err error) { - tlen := len(tstr) - if tlen < minLen || tlen > maxLen { - err = ErrInvalidTime - return - } - - if tstr == isoNullTime[:tlen] { - // This is what MySQL would send when the date is NULL, - // so return an empty time.Time instead. - // This is not a parsing error - return - } - - if loc == nil { - loc = time.UTC - } - - // Since the time format returned from MySQL never has a Timezone - // section, ParseInLocation will initialize the time.Time struct - // with the default `loc` we're passing here. - return time.ParseInLocation(isoTimeFormat[:tlen], tstr, loc) -} - -// datetimeToNative converts a Datetime Value into a time.Time -func datetimeToNative(v Value, loc *time.Location) (time.Time, error) { - // Valid format string offsets for a DATETIME - // |DATETIME |19+ - // |------------------|------| - // "2006-01-02 15:04:05.999999" - return parseISOTime(v.ToString(), loc, 19, isoTimeLength) -} - -// dateToNative converts a Date Value into a time.Time. -// Note that there's no specific type in the Go stdlib to represent -// dates without time components, so the returned Time will have -// their hours/mins/seconds zeroed out. -func dateToNative(v Value, loc *time.Location) (time.Time, error) { - // Valid format string offsets for a DATE - // |DATE |10 - // |---------| - // "2006-01-02 00:00:00.000000" - return parseISOTime(v.ToString(), loc, 10, 10) -} - // EncodeSQL encodes the value into an SQL statement. Can be binary. func (v Value) EncodeSQL(b BinWriter) { switch { diff --git a/go/sqltypes/value_test.go b/go/sqltypes/value_test.go index 99f8566472e..f6360c8b5c4 100644 --- a/go/sqltypes/value_test.go +++ b/go/sqltypes/value_test.go @@ -18,7 +18,6 @@ package sqltypes import ( "math" - "reflect" "strings" "testing" "time" @@ -623,145 +622,6 @@ func DateValue(str string) Value { return TestValue(Date, str) } -func TestDatetimeToNative(t *testing.T) { - tcases := []struct { - val Value - loc *time.Location - out time.Time - err bool - }{{ - val: DatetimeValue("1899-08-24 17:20:00"), - out: time.Date(1899, 8, 24, 17, 20, 0, 0, time.UTC), - }, { - val: DatetimeValue("1952-03-11 01:02:03"), - loc: time.Local, - out: time.Date(1952, 3, 11, 1, 2, 3, 0, time.Local), - }, { - val: DatetimeValue("1952-03-11 01:02:03"), - loc: randomLocation, - out: time.Date(1952, 3, 11, 1, 2, 3, 0, randomLocation), - }, { - val: DatetimeValue("1952-03-11 01:02:03"), - loc: time.UTC, - out: time.Date(1952, 3, 11, 1, 2, 3, 0, time.UTC), - }, { - val: DatetimeValue("1899-08-24 17:20:00.000000"), - out: time.Date(1899, 8, 24, 17, 20, 0, 0, time.UTC), - }, { - val: DatetimeValue("1899-08-24 17:20:00.000001"), - out: time.Date(1899, 8, 24, 17, 20, 0, int(1*time.Microsecond), time.UTC), - }, { - val: DatetimeValue("1899-08-24 17:20:00.123456"), - out: time.Date(1899, 8, 24, 17, 20, 0, int(123456*time.Microsecond), time.UTC), - }, { - val: DatetimeValue("1899-08-24 17:20:00.222"), - out: time.Date(1899, 8, 24, 17, 20, 0, int(222*time.Millisecond), time.UTC), - }, { - val: DatetimeValue("1899-08-24 17:20:00.1234567"), - err: true, - }, { - val: DatetimeValue("1899-08-24 17:20:00.1"), - out: time.Date(1899, 8, 24, 17, 20, 0, int(100*time.Millisecond), time.UTC), - }, { - val: DatetimeValue("0000-00-00 00:00:00"), - out: time.Time{}, - }, { - val: DatetimeValue("0000-00-00 00:00:00.0"), - out: time.Time{}, - }, { - val: DatetimeValue("0000-00-00 00:00:00.000"), - out: time.Time{}, - }, { - val: DatetimeValue("0000-00-00 00:00:00.000000"), - out: time.Time{}, - }, { - val: DatetimeValue("0000-00-00 00:00:00.0000000"), - err: true, - }, { - val: DatetimeValue("1899-08-24T17:20:00.000000"), - err: true, - }, { - val: DatetimeValue("1899-02-31 17:20:00.000000"), - err: true, - }, { - val: DatetimeValue("1899-08-24 17:20:00."), - out: time.Date(1899, 8, 24, 17, 20, 0, 0, time.UTC), - }, { - val: DatetimeValue("0000-00-00 00:00:00.000001"), - err: true, - }, { - val: DatetimeValue("1899-08-24 17:20:00 +02:00"), - err: true, - }, { - val: DatetimeValue("1899-08-24"), - err: true, - }, { - val: DatetimeValue("This is not a valid timestamp"), - err: true, - }} - - for _, tcase := range tcases { - got, err := datetimeToNative(tcase.val, tcase.loc) - if tcase.err && err == nil { - t.Errorf("datetimeToNative(%v, %#v) succeeded; expected error", tcase.val, tcase.loc) - } - if !tcase.err && err != nil { - t.Errorf("datetimeToNative(%v, %#v) failed: %v", tcase.val, tcase.loc, err) - } - if !reflect.DeepEqual(got, tcase.out) { - t.Errorf("datetimeToNative(%v, %#v): %v, want %v", tcase.val, tcase.loc, got, tcase.out) - } - } -} - -func TestDateToNative(t *testing.T) { - tcases := []struct { - val Value - loc *time.Location - out time.Time - err bool - }{{ - val: DateValue("1899-08-24"), - out: time.Date(1899, 8, 24, 0, 0, 0, 0, time.UTC), - }, { - val: DateValue("1952-03-11"), - loc: time.Local, - out: time.Date(1952, 3, 11, 0, 0, 0, 0, time.Local), - }, { - val: DateValue("1952-03-11"), - loc: randomLocation, - out: time.Date(1952, 3, 11, 0, 0, 0, 0, randomLocation), - }, { - val: DateValue("0000-00-00"), - out: time.Time{}, - }, { - val: DateValue("1899-02-31"), - err: true, - }, { - val: DateValue("1899-08-24 17:20:00"), - err: true, - }, { - val: DateValue("0000-00-00 00:00:00"), - err: true, - }, { - val: DateValue("This is not a valid timestamp"), - err: true, - }} - - for _, tcase := range tcases { - got, err := dateToNative(tcase.val, tcase.loc) - if tcase.err && err == nil { - t.Errorf("dateToNative(%v, %#v) succeeded; expected error", tcase.val, tcase.loc) - } - if !tcase.err && err != nil { - t.Errorf("dateToNative(%v, %#v) failed: %v", tcase.val, tcase.loc, err) - } - if !reflect.DeepEqual(got, tcase.out) { - t.Errorf("dateToNative(%v, %#v): %v, want %v", tcase.val, tcase.loc, got, tcase.out) - } - } -} - func TestEncodeSQLStringBuilder(t *testing.T) { testcases := []struct { in Value diff --git a/go/vt/vitessdriver/convert.go b/go/vt/vitessdriver/convert.go index aa8bcedc7ee..c5141d1c890 100644 --- a/go/vt/vitessdriver/convert.go +++ b/go/vt/vitessdriver/convert.go @@ -18,9 +18,11 @@ package vitessdriver import ( "database/sql/driver" + "errors" "fmt" "time" + "vitess.io/vitess/go/mysql/datetime" "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" @@ -43,8 +45,10 @@ func (cv *converter) ToNative(v sqltypes.Value) (any, error) { return v.ToUint64() case v.IsFloat(): return v.ToFloat64() - case v.Type() == sqltypes.Datetime, v.Type() == sqltypes.Timestamp, v.Type() == sqltypes.Date: - return v.ToTimeInLocation(cv.location) + case v.Type() == sqltypes.Datetime, v.Type() == sqltypes.Timestamp: + return cv.datetimeToTime(v) + case v.Type() == sqltypes.Date: + return cv.dateToTime(v) case v.IsQuoted() || v.Type() == sqltypes.Bit || v.Type() == sqltypes.Decimal: out, err = v.ToBytes() case v.Type() == sqltypes.Expression: @@ -53,6 +57,56 @@ func (cv *converter) ToNative(v sqltypes.Value) (any, error) { return out, err } +// ErrInvalidTime is returned when we fail to parse a datetime +// string from MySQL. This should never happen unless things are +// seriously messed up. +var ErrInvalidTime = errors.New("invalid MySQL time string") + +func (cv *converter) datetimeToTime(v sqltypes.Value) (time.Time, error) { + if v.IsNull() { + return time.Time{}, nil + } + // Valid format string offsets for a DATETIME + // |DATETIME |19+ + // |------------------|------| + // "2006-01-02 15:04:05.999999" + dt, _, ok := datetime.ParseDateTime(v.ToString(), -1) + if !ok { + return time.Time{}, ErrInvalidTime + } + if dt.IsZero() { + return time.Time{}, nil + } + loc := cv.location + if loc == nil { + loc = time.UTC + } + return time.Date(dt.Date.Year(), time.Month(dt.Date.Month()), dt.Date.Day(), + dt.Time.Hour(), dt.Time.Minute(), dt.Time.Second(), dt.Time.Nanosecond(), loc), nil +} + +func (cv *converter) dateToTime(v sqltypes.Value) (time.Time, error) { + if v.IsNull() { + return time.Time{}, nil + } + // Valid format string offsets for a DATE + // |DATE |10 + // |---------| + // "2006-01-02 00:00:00.000000" + d, ok := datetime.ParseDate(v.ToString()) + if !ok { + return time.Time{}, ErrInvalidTime + } + if d.IsZero() { + return time.Time{}, nil + } + loc := cv.location + if loc == nil { + loc = time.UTC + } + return time.Date(d.Year(), time.Month(d.Month()), d.Day(), 0, 0, 0, 0, loc), nil +} + func (cv *converter) BuildBindVariable(v any) (*querypb.BindVariable, error) { if t, ok := v.(time.Time); ok { return sqltypes.ValueBindVariable(NewDatetime(t, cv.location)), nil