From a75cc532169db29d9420e00601c9632cf716894d Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 5 Feb 2024 14:38:28 +0100 Subject: [PATCH 1/2] mysql/datetime: Improve TIME parsing logic In places inside the `evalengine` we need to know if a `TIME` instance was properly parsed, if a value could be extracted from input (but it wasn't entirely valid) or if there's enirely invalid input. Before we only had one flag that indicated the strict parsing. So we couldn't really discern partial parsing success from entire failure. We tried that with checking for a zero time, but that's wrong. A zero time is still an entirely valid time and can be the result of successful partial parsing. Signed-off-by: Dirkjan Bussink --- go/mysql/datetime/parse.go | 101 +++++++++++++------ go/mysql/datetime/parse_test.go | 88 +++++++--------- go/mysql/json/parser.go | 4 +- go/vt/sqlparser/literal.go | 4 +- go/vt/sqlparser/normalizer.go | 2 +- go/vt/vtgate/evalengine/compiler_test.go | 4 + go/vt/vtgate/evalengine/eval_temporal.go | 8 +- go/vt/vtgate/evalengine/testcases/cases.go | 19 ++++ go/vt/vtgate/evalengine/testcases/helpers.go | 8 +- 9 files changed, 143 insertions(+), 95 deletions(-) diff --git a/go/mysql/datetime/parse.go b/go/mysql/datetime/parse.go index e8f17191f4c..0d9e6bb2326 100644 --- a/go/mysql/datetime/parse.go +++ b/go/mysql/datetime/parse.go @@ -24,42 +24,45 @@ import ( "vitess.io/vitess/go/mysql/fastparse" ) -func parsetimeHours(tp *timeparts, in string) (out string, ok bool) { +func parsetimeHours(tp *timeparts, in string) (string, TimeState) { + var ok bool if tp.hour, in, ok = getnumn(in); ok { tp.day = tp.day + tp.hour/24 tp.hour = tp.hour % 24 switch { case len(in) == 0: - return "", true + return "", TimeOK case in[0] == ':': return parsetimeMinutes(tp, in[1:]) } } - return "", false + return "", TimePartial } -func parsetimeMinutes(tp *timeparts, in string) (out string, ok bool) { +func parsetimeMinutes(tp *timeparts, in string) (string, TimeState) { + var ok bool if tp.min, in, ok = getnum(in, false); ok { switch { case tp.min > 59: - return "", false + return "", TimeInvalid case len(in) == 0: - return "", true + return "", TimeOK case in[0] == ':': return parsetimeSeconds(tp, in[1:]) } } - return "", false + return "", TimePartial } -func parsetimeSeconds(tp *timeparts, in string) (out string, ok bool) { +func parsetimeSeconds(tp *timeparts, in string) (string, TimeState) { + var ok bool if tp.sec, in, ok = getnum(in, false); ok { switch { case tp.sec > 59: - return "", false + return "", TimeInvalid case len(in) == 0: - return "", true + return "", TimeOK case len(in) > 1 && in[0] == '.': n := 1 for ; n < len(in) && isDigit(in, n); n++ { @@ -67,14 +70,18 @@ func parsetimeSeconds(tp *timeparts, in string) (out string, ok bool) { var l int tp.nsec, l, ok = parseNanoseconds(in, n) tp.prec = uint8(l) - return "", ok && len(in) == n + if ok && len(in) == n { + return "", TimeOK + } + return "", TimePartial } } - return "", false + return "", TimePartial } -func parsetimeAny(tp *timeparts, in string) (out string, ok bool) { +func parsetimeAny(tp *timeparts, in string) (out string, state TimeState) { orig := in + var ok bool for i := 0; i < len(in); i++ { switch r := in[i]; { case isSpace(r): @@ -91,7 +98,7 @@ func parsetimeAny(tp *timeparts, in string) (out string, ok bool) { return parsetimeNoDelimiters(tp, orig) } if tp.day > 34 { - return "", clampTimeparts(tp) + return "", clampTimeparts(tp, state) } return parsetimeHours(tp, in) case r == ':': @@ -101,8 +108,9 @@ func parsetimeAny(tp *timeparts, in string) (out string, ok bool) { return parsetimeNoDelimiters(tp, in) } -func parsetimeNoDelimiters(tp *timeparts, in string) (out string, ok bool) { +func parsetimeNoDelimiters(tp *timeparts, in string) (out string, state TimeState) { var integral int + var ok bool for ; integral < len(in); integral++ { if in[integral] == '.' || !isDigit(in, integral) { break @@ -112,12 +120,9 @@ func parsetimeNoDelimiters(tp *timeparts, in string) (out string, ok bool) { switch integral { default: // MySQL limits this to a numeric value that fits in a 32-bit unsigned integer. - i, _ := fastparse.ParseInt64(in[:integral], 10) + i, _ := fastparse.ParseUint64(in[:integral], 10) if i > math.MaxUint32 { - return "", false - } - if i < -math.MaxUint32 { - return "", false + return "", TimeInvalid } tp.hour, in, ok = getnuml(in, integral-4) @@ -132,7 +137,7 @@ func parsetimeNoDelimiters(tp *timeparts, in string) (out string, ok bool) { case 3, 4: tp.min, in, ok = getnuml(in, integral-2) if !ok || tp.min > 59 { - return "", false + return "", TimeInvalid } integral = 2 fallthrough @@ -140,10 +145,10 @@ func parsetimeNoDelimiters(tp *timeparts, in string) (out string, ok bool) { case 1, 2: tp.sec, in, ok = getnuml(in, integral) if !ok || tp.sec > 59 { - return "", false + return "", TimeInvalid } case 0: - return "", false + return "", TimeInvalid } if len(in) > 1 && in[0] == '.' && isDigit(in, 1) { @@ -152,14 +157,18 @@ func parsetimeNoDelimiters(tp *timeparts, in string) (out string, ok bool) { } var l int tp.nsec, l, ok = parseNanoseconds(in, n) + if !ok { + state = TimeInvalid + } tp.prec = uint8(l) in = in[n:] } - return in, clampTimeparts(tp) && ok + state = clampTimeparts(tp, state) + return in, state } -func clampTimeparts(tp *timeparts) bool { +func clampTimeparts(tp *timeparts, state TimeState) TimeState { // Maximum time is 838:59:59, so we have to clamp // it to that value here if we otherwise successfully // parser the time. @@ -168,15 +177,31 @@ func clampTimeparts(tp *timeparts) bool { tp.hour = 22 tp.min = 59 tp.sec = 59 - return false + if state == TimeOK { + return TimePartial + } } - return true + return state } -func ParseTime(in string, prec int) (t Time, l int, ok bool) { +type TimeState uint8 + +const ( + // TimeOK indicates that the parsed value is valid and complete. + TimeOK TimeState = iota + // TimePartial indicates that the parsed value has a partially parsed value + // but it is not fully complete and valid. There could be additional stray + // data in the input, or it has an overflow. + TimePartial + // TimeInvalid indicates that the parsed value is invalid and no partial + // TIME value could be extracted from the input. + TimeInvalid +) + +func ParseTime(in string, prec int) (t Time, l int, state TimeState) { in = strings.Trim(in, " \t\r\n") if len(in) == 0 { - return Time{}, 0, false + return Time{}, 0, TimeInvalid } var neg bool if in[0] == '-' { @@ -185,11 +210,15 @@ func ParseTime(in string, prec int) (t Time, l int, ok bool) { } var tp timeparts - in, ok = parsetimeAny(&tp, in) - ok = clampTimeparts(&tp) && ok + in, state = parsetimeAny(&tp, in) + if state == TimeInvalid { + return Time{}, 0, state + } + + state = clampTimeparts(&tp, state) hours := uint16(24*tp.day + tp.hour) - if !tp.isZero() && neg { + if neg { hours |= negMask } @@ -206,7 +235,13 @@ func ParseTime(in string, prec int) (t Time, l int, ok bool) { t = t.Round(prec) } - return t, prec, ok && len(in) == 0 + switch { + case state == TimeOK && len(in) == 0: + state = TimeOK + case state == TimeOK && len(in) > 0: + state = TimePartial + } + return t, prec, state } func ParseDate(s string) (Date, bool) { diff --git a/go/mysql/datetime/parse_test.go b/go/mysql/datetime/parse_test.go index 6ed342edfb3..66fb8a73b2f 100644 --- a/go/mysql/datetime/parse_test.go +++ b/go/mysql/datetime/parse_test.go @@ -96,32 +96,33 @@ func TestParseTime(t *testing.T) { output testTime norm string l int - err bool + state TimeState }{ {input: "00:00:00", norm: "00:00:00.000000", output: testTime{}}, - {input: "00:00:00foo", norm: "00:00:00.000000", output: testTime{}, err: true}, + {input: "-00:00:00", norm: "-00:00:00.000000", output: testTime{negative: true}}, + {input: "00:00:00foo", norm: "00:00:00.000000", output: testTime{}, state: TimePartial}, {input: "11:12:13", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}}, - {input: "11:12:13foo", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}, err: true}, + {input: "11:12:13foo", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}, state: TimePartial}, {input: "11:12:13.1", norm: "11:12:13.100000", output: testTime{11, 12, 13, 100000000, false}, l: 1}, - {input: "11:12:13.foo", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}, err: true}, - {input: "11:12:13.1foo", norm: "11:12:13.100000", output: testTime{11, 12, 13, 100000000, false}, l: 1, err: true}, + {input: "11:12:13.foo", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}, state: TimePartial}, + {input: "11:12:13.1foo", norm: "11:12:13.100000", output: testTime{11, 12, 13, 100000000, false}, l: 1, state: TimePartial}, {input: "11:12:13.123456", norm: "11:12:13.123456", output: testTime{11, 12, 13, 123456000, false}, l: 6}, {input: "11:12:13.000001", norm: "11:12:13.000001", output: testTime{11, 12, 13, 1000, false}, l: 6}, {input: "11:12:13.000000", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}, l: 6}, - {input: "11:12:13.123456foo", norm: "11:12:13.123456", output: testTime{11, 12, 13, 123456000, false}, l: 6, err: true}, + {input: "11:12:13.123456foo", norm: "11:12:13.123456", output: testTime{11, 12, 13, 123456000, false}, l: 6, state: TimePartial}, {input: "3 11:12:13", norm: "83:12:13.000000", output: testTime{3*24 + 11, 12, 13, 0, false}}, - {input: "3 11:12:13foo", norm: "83:12:13.000000", output: testTime{3*24 + 11, 12, 13, 0, false}, err: true}, + {input: "3 11:12:13foo", norm: "83:12:13.000000", output: testTime{3*24 + 11, 12, 13, 0, false}, state: TimePartial}, {input: "3 41:12:13", norm: "113:12:13.000000", output: testTime{3*24 + 41, 12, 13, 0, false}}, - {input: "3 41:12:13foo", norm: "113:12:13.000000", output: testTime{3*24 + 41, 12, 13, 0, false}, err: true}, - {input: "34 23:12:13", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: "35 11:12:13", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, + {input: "3 41:12:13foo", norm: "113:12:13.000000", output: testTime{3*24 + 41, 12, 13, 0, false}, state: TimePartial}, + {input: "34 23:12:13", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, + {input: "35 11:12:13", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, {input: "11:12", norm: "11:12:00.000000", output: testTime{11, 12, 0, 0, false}}, {input: "5 11:12", norm: "131:12:00.000000", output: testTime{5*24 + 11, 12, 0, 0, false}}, {input: "-2 11:12", norm: "-59:12:00.000000", output: testTime{2*24 + 11, 12, 0, 0, true}}, - {input: "--2 11:12", norm: "00:00:00.000000", err: true}, - {input: "nonsense", norm: "00:00:00.000000", err: true}, + {input: "--2 11:12", norm: "00:00:00.000000", state: TimeInvalid}, + {input: "nonsense", norm: "00:00:00.000000", state: TimeInvalid}, {input: "2 11", norm: "59:00:00.000000", output: testTime{2*24 + 11, 0, 0, 0, false}}, - {input: "2 -11", norm: "00:00:02.000000", output: testTime{0, 0, 2, 0, false}, err: true}, + {input: "2 -11", norm: "00:00:02.000000", output: testTime{0, 0, 2, 0, false}, state: TimePartial}, {input: "13", norm: "00:00:13.000000", output: testTime{0, 0, 13, 0, false}}, {input: "111213", norm: "11:12:13.000000", output: testTime{11, 12, 13, 0, false}}, {input: "111213.123456", norm: "11:12:13.123456", output: testTime{11, 12, 13, 123456000, false}, l: 6}, @@ -130,19 +131,21 @@ func TestParseTime(t *testing.T) { {input: "25:12:13", norm: "25:12:13.000000", output: testTime{25, 12, 13, 0, false}}, {input: "32:35", norm: "32:35:00.000000", output: testTime{32, 35, 0, 0, false}}, {input: "101:34:58", norm: "101:34:58.000000", output: testTime{101, 34, 58, 0, false}}, + {input: "101:64:58", norm: "00:00:00.000000", state: TimeInvalid}, + {input: "101:34:68", norm: "00:00:00.000000", state: TimeInvalid}, {input: "1", norm: "00:00:01.000000", output: testTime{0, 0, 1, 0, false}}, {input: "11", norm: "00:00:11.000000", output: testTime{0, 0, 11, 0, false}}, {input: "111", norm: "00:01:11.000000", output: testTime{0, 1, 11, 0, false}}, {input: "1111", norm: "00:11:11.000000", output: testTime{0, 11, 11, 0, false}}, {input: "11111", norm: "01:11:11.000000", output: testTime{1, 11, 11, 0, false}}, {input: "111111", norm: "11:11:11.000000", output: testTime{11, 11, 11, 0, false}}, - {input: "1foo", norm: "00:00:01.000000", output: testTime{0, 0, 1, 0, false}, err: true}, - {input: "11foo", norm: "00:00:11.000000", output: testTime{0, 0, 11, 0, false}, err: true}, - {input: "111foo", norm: "00:01:11.000000", output: testTime{0, 1, 11, 0, false}, err: true}, - {input: "1111foo", norm: "00:11:11.000000", output: testTime{0, 11, 11, 0, false}, err: true}, - {input: "11111foo", norm: "01:11:11.000000", output: testTime{1, 11, 11, 0, false}, err: true}, - {input: "111111foo", norm: "11:11:11.000000", output: testTime{11, 11, 11, 0, false}, err: true}, - {input: "1111111foo", norm: "111:11:11.000000", output: testTime{111, 11, 11, 0, false}, err: true}, + {input: "1foo", norm: "00:00:01.000000", output: testTime{0, 0, 1, 0, false}, state: TimePartial}, + {input: "11foo", norm: "00:00:11.000000", output: testTime{0, 0, 11, 0, false}, state: TimePartial}, + {input: "111foo", norm: "00:01:11.000000", output: testTime{0, 1, 11, 0, false}, state: TimePartial}, + {input: "1111foo", norm: "00:11:11.000000", output: testTime{0, 11, 11, 0, false}, state: TimePartial}, + {input: "11111foo", norm: "01:11:11.000000", output: testTime{1, 11, 11, 0, false}, state: TimePartial}, + {input: "111111foo", norm: "11:11:11.000000", output: testTime{11, 11, 11, 0, false}, state: TimePartial}, + {input: "1111111foo", norm: "111:11:11.000000", output: testTime{111, 11, 11, 0, false}, state: TimePartial}, {input: "-1", norm: "-00:00:01.000000", output: testTime{0, 0, 1, 0, true}}, {input: "-11", norm: "-00:00:11.000000", output: testTime{0, 0, 11, 0, true}}, {input: "-111", norm: "-00:01:11.000000", output: testTime{0, 1, 11, 0, true}}, @@ -172,44 +175,31 @@ func TestParseTime(t *testing.T) { {input: "11111.1", norm: "01:11:11.100000", output: testTime{1, 11, 11, 100000000, false}, l: 1}, {input: "111111.1", norm: "11:11:11.100000", output: testTime{11, 11, 11, 100000000, false}, l: 1}, {input: "1111111.1", norm: "111:11:11.100000", output: testTime{111, 11, 11, 100000000, false}, l: 1}, - {input: "20000101", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: "-20000101", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, err: true}, - {input: "999995959", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: "-999995959", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, err: true}, - {input: "4294965959", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: "-4294965959", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, err: true}, - {input: "4294975959", norm: "00:00:00.000000", err: true}, - {input: "-4294975959", norm: "00:00:00.000000", err: true}, - {input: "\t34 foo\t", norm: "00:00:34.000000", output: testTime{0, 0, 34, 0, false}, err: true}, - {input: "\t34 1foo\t", norm: "817:00:00.000000", output: testTime{817, 0, 0, 0, false}, err: true}, - {input: "\t34 23foo\t", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: "\t35 foo\t", norm: "00:00:35.000000", output: testTime{0, 0, 35, 0, false}, err: true}, - {input: "\t35 1foo\t", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, err: true}, - {input: " 255 foo", norm: "00:02:55.000000", output: testTime{0, 2, 55, 0, false}, err: true}, - {input: "255", norm: "00:02:55.000000", output: testTime{0, 2, 55, 0, false}}, + {input: "20000101", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, + {input: "-20000101", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, state: TimePartial}, + {input: "999995959", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, + {input: "-999995959", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, state: TimePartial}, + {input: "4294965959", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, + {input: "-4294965959", norm: "-838:59:59.000000", output: testTime{838, 59, 59, 0, true}, state: TimePartial}, + {input: "4294975959", norm: "00:00:00.000000", state: TimeInvalid}, + {input: "-4294975959", norm: "00:00:00.000000", state: TimeInvalid}, + {input: "\t34 foo\t", norm: "00:00:34.000000", output: testTime{0, 0, 34, 0, false}, state: TimePartial}, + {input: "\t34 1foo\t", norm: "817:00:00.000000", output: testTime{817, 0, 0, 0, false}, state: TimePartial}, + {input: "\t34 23foo\t", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, + {input: "\t35 foo\t", norm: "00:00:35.000000", output: testTime{0, 0, 35, 0, false}, state: TimePartial}, + {input: "\t35 1foo\t", norm: "838:59:59.000000", output: testTime{838, 59, 59, 0, false}, state: TimePartial}, } for _, test := range tests { t.Run(test.input, func(t *testing.T) { - got, l, ok := ParseTime(test.input, -1) - if test.err { - assert.Equal(t, test.output.hour, got.Hour()) - assert.Equal(t, test.output.minute, got.Minute()) - assert.Equal(t, test.output.second, got.Second()) - assert.Equal(t, test.output.nanosecond, got.Nanosecond()) - assert.Equal(t, test.norm, string(got.AppendFormat(nil, 6))) - assert.Equal(t, test.l, l) - assert.Falsef(t, ok, "did not fail to parse %s", test.input) - return - } - - require.True(t, ok) + got, l, state := ParseTime(test.input, -1) + assert.Equal(t, test.state, state) assert.Equal(t, test.output.hour, got.Hour()) assert.Equal(t, test.output.minute, got.Minute()) assert.Equal(t, test.output.second, got.Second()) assert.Equal(t, test.output.nanosecond, got.Nanosecond()) - assert.Equal(t, test.l, l) assert.Equal(t, test.norm, string(got.AppendFormat(nil, 6))) + assert.Equal(t, test.l, l) }) } } diff --git a/go/mysql/json/parser.go b/go/mysql/json/parser.go index 35278263877..707d890df93 100644 --- a/go/mysql/json/parser.go +++ b/go/mysql/json/parser.go @@ -941,8 +941,8 @@ func (v *Value) Time() (datetime.Time, bool) { if v.t != TypeTime { return datetime.Time{}, false } - t, _, ok := datetime.ParseTime(v.s, datetime.DefaultPrecision) - return t, ok + t, _, state := datetime.ParseTime(v.s, datetime.DefaultPrecision) + return t, state == datetime.TimeOK } // Object returns the underlying JSON object for the v. diff --git a/go/vt/sqlparser/literal.go b/go/vt/sqlparser/literal.go index 71fed3d7d16..bde53798a19 100644 --- a/go/vt/sqlparser/literal.go +++ b/go/vt/sqlparser/literal.go @@ -87,8 +87,8 @@ func LiteralToValue(lit *Literal) (sqltypes.Value, error) { buf := datetime.Date_YYYY_MM_DD.Format(datetime.DateTime{Date: d}, 0) return sqltypes.NewDate(hack.String(buf)), nil case TimeVal: - t, l, ok := datetime.ParseTime(lit.Val, -1) - if !ok { + t, l, state := datetime.ParseTime(lit.Val, -1) + if state != datetime.TimeOK { return sqltypes.Value{}, fmt.Errorf("invalid time literal: %v", lit.Val) } buf := datetime.Time_hh_mm_ss.Format(datetime.DateTime{Time: t}, uint8(l)) diff --git a/go/vt/sqlparser/normalizer.go b/go/vt/sqlparser/normalizer.go index b1728a47fb1..0254dccdfb2 100644 --- a/go/vt/sqlparser/normalizer.go +++ b/go/vt/sqlparser/normalizer.go @@ -165,7 +165,7 @@ func validateLiteral(node *Literal) error { return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Incorrect DATE value: '%s'", node.Val) } case TimeVal: - if _, _, ok := datetime.ParseTime(node.Val, -1); !ok { + if _, _, state := datetime.ParseTime(node.Val, -1); state != datetime.TimeOK { return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Incorrect TIME value: '%s'", node.Val) } case TimestampVal: diff --git a/go/vt/vtgate/evalengine/compiler_test.go b/go/vt/vtgate/evalengine/compiler_test.go index 537fc64be3c..7b2c92783ee 100644 --- a/go/vt/vtgate/evalengine/compiler_test.go +++ b/go/vt/vtgate/evalengine/compiler_test.go @@ -611,6 +611,10 @@ func TestCompilerSingle(t *testing.T) { expression: `now() + interval 654321 microsecond`, result: `DATETIME("2023-10-24 12:00:00.654321")`, }, + { + expression: `time('1111:66:56')`, + result: `NULL`, + }, } tz, _ := time.LoadLocation("Europe/Madrid") diff --git a/go/vt/vtgate/evalengine/eval_temporal.go b/go/vt/vtgate/evalengine/eval_temporal.go index fec310f5bf7..7706ec36e64 100644 --- a/go/vt/vtgate/evalengine/eval_temporal.go +++ b/go/vt/vtgate/evalengine/eval_temporal.go @@ -203,8 +203,8 @@ func parseDateTime(s []byte) (*evalTemporal, error) { } func parseTime(s []byte) (*evalTemporal, error) { - t, l, ok := datetime.ParseTime(hack.String(s), -1) - if !ok { + t, l, state := datetime.ParseTime(hack.String(s), -1) + if state != datetime.TimeOK { return nil, errIncorrectTemporal("TIME", s) } return newEvalTime(t, l), nil @@ -228,7 +228,7 @@ func evalToTemporal(e eval, allowZero bool) *evalTemporal { if d, ok := datetime.ParseDate(e.string()); ok { return newEvalDate(d, allowZero) } - if t, l, ok := datetime.ParseTime(e.string(), -1); ok { + if t, l, state := datetime.ParseTime(e.string(), -1); state == datetime.TimeOK { return newEvalTime(t, l) } case *evalInt64: @@ -293,7 +293,7 @@ func evalToTime(e eval, l int) *evalTemporal { if dt, l, _ := datetime.ParseDateTime(e.string(), l); !dt.IsZero() { return newEvalTime(dt.Time, l) } - if t, l, ok := datetime.ParseTime(e.string(), l); ok || !t.IsZero() { + if t, l, state := datetime.ParseTime(e.string(), l); state != datetime.TimeInvalid { return newEvalTime(t, l) } case *evalInt64: diff --git a/go/vt/vtgate/evalengine/testcases/cases.go b/go/vt/vtgate/evalengine/testcases/cases.go index b9ae41722eb..b0d560ef206 100644 --- a/go/vt/vtgate/evalengine/testcases/cases.go +++ b/go/vt/vtgate/evalengine/testcases/cases.go @@ -1857,6 +1857,25 @@ func FnTime(yield Query) { for _, d := range inputConversions { yield(fmt.Sprintf("TIME(%s)", d), nil) } + times := []string{ + "'00:00:00'", + "'asdadsasd'", + "'312sadd'", + "'11-12-23'", + "'0000-11-23'", + "'0-0-0'", + "00:00", + "00:00-00", + "00:00:0:0:0:0", + "00::00", + "12::00", + "'00000001'", + "'11116656'", + } + + for _, d := range times { + yield(fmt.Sprintf("TIME(%s)", d), nil) + } } func FnUnixTimestamp(yield Query) { diff --git a/go/vt/vtgate/evalengine/testcases/helpers.go b/go/vt/vtgate/evalengine/testcases/helpers.go index 245d59992aa..a908b8196c8 100644 --- a/go/vt/vtgate/evalengine/testcases/helpers.go +++ b/go/vt/vtgate/evalengine/testcases/helpers.go @@ -187,12 +187,12 @@ func (cmp *Comparison) Equals(local, remote sqltypes.Value, now time.Time) (bool } return cmp.closeDatetime(localDatetime.ToStdTime(now), remoteDatetime.ToStdTime(now), 1*time.Second), nil case cmp.LooseTime && local.IsTime() && remote.IsTime(): - localTime, _, ok := datetime.ParseTime(local.ToString(), -1) - if !ok { + localTime, _, state := datetime.ParseTime(local.ToString(), -1) + if state != datetime.TimeOK { return false, fmt.Errorf("error converting local value '%s' to time", local) } - remoteTime, _, ok := datetime.ParseTime(remote.ToString(), -1) - if !ok { + remoteTime, _, state := datetime.ParseTime(remote.ToString(), -1) + if state != datetime.TimeOK { return false, fmt.Errorf("error converting remote value '%s' to time", remote) } return cmp.closeDatetime(localTime.ToStdTime(now), remoteTime.ToStdTime(now), 1*time.Second), nil From 92f8b8827ae3f06ee67359f03c04f88b384e9576 Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 5 Feb 2024 15:21:14 +0100 Subject: [PATCH 2/2] datetime: Fix YEARWEEK Signed-off-by: Dirkjan Bussink --- go/mysql/datetime/datetime.go | 9 ++++++--- go/vt/vtgate/evalengine/testcases/cases.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/go/mysql/datetime/datetime.go b/go/mysql/datetime/datetime.go index c27593ee0a6..7a532818cc8 100644 --- a/go/mysql/datetime/datetime.go +++ b/go/mysql/datetime/datetime.go @@ -368,9 +368,12 @@ func (d Date) YearWeek(mode int) int { case 1, 3: year, week := d.ISOWeek() return year*100 + week - case 4, 5, 6, 7: - // TODO - return 0 + case 4, 6: + year, week := d.Sunday4DayWeek() + return year*100 + week + case 5, 7: + year, week := d.MondayWeek() + return year*100 + week default: return d.YearWeek(DefaultWeekMode) } diff --git a/go/vt/vtgate/evalengine/testcases/cases.go b/go/vt/vtgate/evalengine/testcases/cases.go index b0d560ef206..77bd534ef2d 100644 --- a/go/vt/vtgate/evalengine/testcases/cases.go +++ b/go/vt/vtgate/evalengine/testcases/cases.go @@ -1917,7 +1917,7 @@ func FnYear(yield Query) { } func FnYearWeek(yield Query) { - for i := 0; i < 4; i++ { + for i := 0; i < 8; i++ { for _, d := range inputConversions { yield(fmt.Sprintf("YEARWEEK(%s, %d)", d, i), nil) }