From 8ae81b38525bdbac85d1e1bf6b1b272da6b5ad68 Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Thu, 4 Nov 2021 19:17:02 +0800 Subject: [PATCH] expression: fix different results for greatest when vectorized is off (#29438) --- expression/builtin_compare.go | 33 ++++-------------------------- expression/builtin_compare_test.go | 4 ++-- expression/integration_test.go | 21 +++++++++++++++++-- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/expression/builtin_compare.go b/expression/builtin_compare.go index e5c2dcb08fcfa..a3aa3e085b0c3 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -642,11 +642,7 @@ func (b *builtinGreatestTimeSig) Clone() builtinFunc { // evalString evals a builtinGreatestTimeSig. // See http://dev.mysql.com/doc/refman/5.7/en/comparison-operators.html#function_greatest -func (b *builtinGreatestTimeSig) evalString(row chunk.Row) (res string, isNull bool, err error) { - var ( - strRes string - timeRes types.Time - ) +func (b *builtinGreatestTimeSig) evalString(row chunk.Row) (strRes string, isNull bool, err error) { sc := b.ctx.GetSessionVars().StmtCtx for i := 0; i < len(b.args); i++ { v, isNull, err := b.args[i].EvalString(b.ctx, row) @@ -665,16 +661,8 @@ func (b *builtinGreatestTimeSig) evalString(row chunk.Row) (res string, isNull b if i == 0 || strings.Compare(v, strRes) > 0 { strRes = v } - if i == 0 || t.Compare(timeRes) > 0 { - timeRes = t - } - } - if timeRes.IsZero() { - res = strRes - } else { - res = timeRes.String() } - return res, false, nil + return strRes, false, nil } type leastFunctionClass struct { @@ -860,12 +848,7 @@ func (b *builtinLeastTimeSig) Clone() builtinFunc { // evalString evals a builtinLeastTimeSig. // See http://dev.mysql.com/doc/refman/5.7/en/comparison-operators.html#functionleast -func (b *builtinLeastTimeSig) evalString(row chunk.Row) (res string, isNull bool, err error) { - var ( - // timeRes will be converted to a strRes only when the arguments is a valid datetime value. - strRes string // Record the strRes of each arguments. - timeRes types.Time // Record the time representation of a valid arguments. - ) +func (b *builtinLeastTimeSig) evalString(row chunk.Row) (strRes string, isNull bool, err error) { sc := b.ctx.GetSessionVars().StmtCtx for i := 0; i < len(b.args); i++ { v, isNull, err := b.args[i].EvalString(b.ctx, row) @@ -883,17 +866,9 @@ func (b *builtinLeastTimeSig) evalString(row chunk.Row) (res string, isNull bool if i == 0 || strings.Compare(v, strRes) < 0 { strRes = v } - if i == 0 || t.Compare(timeRes) < 0 { - timeRes = t - } } - if timeRes.IsZero() { - res = strRes - } else { - res = timeRes.String() - } - return res, false, nil + return strRes, false, nil } type intervalFunctionClass struct { diff --git a/expression/builtin_compare_test.go b/expression/builtin_compare_test.go index 9aa5a7a984598..f9ebb4c4c548c 100644 --- a/expression/builtin_compare_test.go +++ b/expression/builtin_compare_test.go @@ -312,11 +312,11 @@ func TestGreatestLeastFunc(t *testing.T) { }, { []interface{}{tm, "invalid_time_1", "invalid_time_2", tmWithFsp}, - curTimeWithFspString, curTimeString, false, false, + "invalid_time_2", curTimeString, false, false, }, { []interface{}{tm, "invalid_time_2", "invalid_time_1", tmWithFsp}, - curTimeWithFspString, curTimeString, false, false, + "invalid_time_2", curTimeString, false, false, }, { []interface{}{tm, "invalid_time", nil, tmWithFsp}, diff --git a/expression/integration_test.go b/expression/integration_test.go index 59984a2c97f61..d071bcc4ec7a1 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -3942,8 +3942,7 @@ func (s *testIntegrationSuite) TestCompareBuiltin(c *C) { result.Check(testkit.Rows("3 c 1.3 2")) tk.MustQuery("show warnings").Check(testkit.Rows()) result = tk.MustQuery(`select greatest(cast("2017-01-01" as datetime), "123", "234", cast("2018-01-01" as date)), greatest(cast("2017-01-01" as date), "123", null)`) - // todo: MySQL returns "2018-01-01 " - result.Check(testkit.Rows("2018-01-01 00:00:00 ")) + result.Check(testkit.Rows("234 ")) tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Incorrect time value: '123'", "Warning|1292|Incorrect time value: '234'", "Warning|1292|Incorrect time value: '123'")) // for least result = tk.MustQuery(`select least(1, 2, 3), least("a", "b", "c"), least(1.1, 1.2, 1.3), least("123a", 1, 2)`) @@ -10433,6 +10432,24 @@ func (s *testIntegrationSuite) TestIssue28643(c *C) { tk.MustQuery("select hour(a) from t;").Check(testkit.Rows("838", "838")) } +func (s *testIntegrationSuite) TestIssue29434(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + + tk.MustExec("drop table if exists t1;") + tk.MustExec("create table t1(c1 datetime);") + tk.MustExec("insert into t1 values('2021-12-12 10:10:10.000');") + tk.MustExec("set tidb_enable_vectorized_expression = on;") + tk.MustQuery("select greatest(c1, '99999999999999') from t1;").Check(testkit.Rows("99999999999999")) + tk.MustExec("set tidb_enable_vectorized_expression = off;") + tk.MustQuery("select greatest(c1, '99999999999999') from t1;").Check(testkit.Rows("99999999999999")) + + tk.MustExec("set tidb_enable_vectorized_expression = on;") + tk.MustQuery("select least(c1, '99999999999999') from t1;").Check(testkit.Rows("2021-12-12 10:10:10")) + tk.MustExec("set tidb_enable_vectorized_expression = off;") + tk.MustQuery("select least(c1, '99999999999999') from t1;").Check(testkit.Rows("2021-12-12 10:10:10")) +} + func (s *testIntegrationSuite) TestIssue29244(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test")