Skip to content

Commit

Permalink
expression: fix different results for greatest when vectorized is off (
Browse files Browse the repository at this point in the history
  • Loading branch information
guo-shaoge authored Nov 4, 2021
1 parent fb5b3a0 commit 8ae81b3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 33 deletions.
33 changes: 4 additions & 29 deletions expression/builtin_compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions expression/builtin_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
21 changes: 19 additions & 2 deletions expression/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <nil>"
result.Check(testkit.Rows("2018-01-01 00:00:00 <nil>"))
result.Check(testkit.Rows("234 <nil>"))
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)`)
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 8ae81b3

Please sign in to comment.