From aec80f9517fdecd36e5e257b4a91a69bf956116f Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Fri, 15 Apr 2022 14:24:36 +0800 Subject: [PATCH] expression: fix different results for greatest when vectorized is off (#29438) (#29919) close pingcap/tidb#29434 --- 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 d28113e51e88c..800c034b1d886 100644 --- a/expression/builtin_compare.go +++ b/expression/builtin_compare.go @@ -634,11 +634,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) @@ -657,16 +653,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 { @@ -852,12 +840,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) @@ -875,17 +858,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 9d11d8b5ad18a..b8d3db9ae9937 100644 --- a/expression/builtin_compare_test.go +++ b/expression/builtin_compare_test.go @@ -296,11 +296,11 @@ func (s *testEvaluatorSuite) TestGreatestLeastFunc(c *C) { }, { []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 5451788a818ab..ad9bb9570268e 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -3916,8 +3916,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)`) @@ -10199,6 +10198,24 @@ func (s *testIntegrationSuite) TestIssue27610(c *C) { Check(testkit.Rows()) } +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) TestIssue29417(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test")