Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expression: Fix incorrect behavior of greatest/least when mixed temporal type with others #31037

Merged
merged 30 commits into from
Dec 30, 2021
Merged

expression: Fix incorrect behavior of greatest/least when mixed temporal type with others #31037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a2dc3eb
Fix greatest/least issue when handling duration type
Dec 21, 2021
29185d6
Fix some greatest/least function incorrect behavior when mixed types
yibin87 Dec 23, 2021
4a29734
Fix greatest/least issue when handling duration type
yibin87 Dec 21, 2021
56e033c
merge changes from trunk
yibin87 Dec 23, 2021
545dec2
Separate GreatestTime and GreatestDate, LeastTime and LeastDate, and …
yibin87 Dec 27, 2021
5ffc3a6
Merge branch 'master' of github.com:yibin87/tidb
yibin87 Dec 27, 2021
a4d5db5
Merge branch 'master' into master
yibin87 Dec 28, 2021
fb273c7
Add support for vector greatest/least functions
yibin87 Dec 28, 2021
2dac27c
Merge branch 'master' of github.com:yibin87/tidb
yibin87 Dec 28, 2021
e1574b1
make format
yibin87 Dec 28, 2021
5c4aef3
Format enum definitions
yibin87 Dec 28, 2021
6ad60a3
Format enum definitions
yibin87 Dec 28, 2021
3003a03
Format enum definitions
yibin87 Dec 28, 2021
9e571ff
Format change
yibin87 Dec 29, 2021
18244f0
Format change
yibin87 Dec 29, 2021
f1fc032
Format change
yibin87 Dec 29, 2021
367ab6e
Refact code and add vec test cases
yibin87 Dec 29, 2021
630983e
Merge branch 'master' into master
yibin87 Dec 29, 2021
b745867
Fmt changes
yibin87 Dec 29, 2021
1e11ca0
Modify comment to trigger unit test
yibin87 Dec 29, 2021
d4505c3
Adjust import sequence
yibin87 Dec 30, 2021
3ba6f5d
Merge branch 'master' into master
ti-chi-bot Dec 30, 2021
de2240f
Merge branch 'master' into master
ti-chi-bot Dec 30, 2021
a6addf6
Merge branch 'master' into master
ti-chi-bot Dec 30, 2021
fa47deb
Merge branch 'master' into master
ti-chi-bot Dec 30, 2021
88fd4d6
Fix mysql test json case
yibin87 Dec 30, 2021
6bbb43d
Merge branch 'master' of github.com:yibin87/tidb
yibin87 Dec 30, 2021
6ba4c28
Merge branch 'master' into master
yibin87 Dec 30, 2021
356c372
Add JSON tests
yibin87 Dec 30, 2021
2fabae4
Merge branch 'master' of github.com:yibin87/tidb
yibin87 Dec 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 134 additions & 35 deletions expression/builtin_compare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pingcap/tidb/parser/opcode"
"github.com/pingcap/tidb/parser/terror"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/types/json"
"github.com/pingcap/tidb/util/chunk"
Expand Down Expand Up @@ -416,10 +417,33 @@ func ResolveType4Between(args [3]Expression) types.EvalType {
return cmpTp
}

// GLCmpStringMode represents Greatest/Least interal string comparison mode
type GLCmpStringMode uint8

const (
// GLCmpStringDirectly Greatest and Least function compares string directly
GLCmpStringDirectly GLCmpStringMode = iota
// GLCmpStringAsDate Greatest/Least function compares string as 'yyyy-mm-dd' format
GLCmpStringAsDate
// GLCmpStringAsDatetime Greatest/Least function compares string as 'yyyy-mm-dd hh:mm:ss' format
GLCmpStringAsDatetime
)

// GLRetTimeType represents Greatest/Least return time type
type GLRetTimeType uint8

const (
// GLRetNoneTemporal Greatest/Least function returns non temporal time
GLRetNoneTemporal GLRetTimeType = iota
// GLRetDate Greatest/Least function returns date type, 'yyyy-mm-dd'
GLRetDate
// GLRetDatetime Greatest/Least function returns datetime type, 'yyyy-mm-dd hh:mm:ss'
GLRetDatetime
)

// resolveType4Extremum gets compare type for GREATEST and LEAST and BETWEEN (mainly for datetime).
func resolveType4Extremum(args []Expression) (_ types.EvalType, cmpStringAsDatetime bool) {
func resolveType4Extremum(args []Expression) (_ types.EvalType, fieldTimeType GLRetTimeType, cmpStringMode GLCmpStringMode) {
aggType := aggregateType(args)

var temporalItem *types.FieldType
if aggType.EvalType().IsStringKind() {
for i := range args {
Expand All @@ -432,13 +456,22 @@ func resolveType4Extremum(args []Expression) (_ types.EvalType, cmpStringAsDatet
}
}

if !types.IsTypeTemporal(aggType.Tp) && temporalItem != nil {
aggType.Tp = temporalItem.Tp
cmpStringAsDatetime = true
if !types.IsTypeTemporal(aggType.Tp) && temporalItem != nil && types.IsTemporalWithDate(temporalItem.Tp) {
if temporalItem.Tp == mysql.TypeDate {
cmpStringMode = GLCmpStringAsDate
} else {
cmpStringMode = GLCmpStringAsDatetime
}
}
// TODO: String charset, collation checking are needed.
}
return aggType.EvalType(), cmpStringAsDatetime
var timeType = GLRetNoneTemporal
if aggType.Tp == mysql.TypeDate {
timeType = GLRetDate
} else if aggType.Tp == mysql.TypeDatetime || aggType.Tp == mysql.TypeTimestamp {
timeType = GLRetDatetime
}
return aggType.EvalType(), timeType, cmpStringMode
}

// unsupportedJSONComparison reports warnings while there is a JSON type in least/greatest function's arguments
Expand All @@ -460,23 +493,25 @@ func (c *greatestFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
if err = c.verifyArgs(args); err != nil {
return nil, err
}
tp, cmpStringAsDatetime := resolveType4Extremum(args)
if cmpStringAsDatetime {
tp, fieldTimeType, cmpStringMode := resolveType4Extremum(args)
argTp := tp
if cmpStringMode != GLCmpStringDirectly {
// Args are temporal and string mixed, we cast all args as string and parse it to temporal mannualy to compare.
tp = types.ETString
argTp = types.ETString
} else if tp == types.ETJson {
unsupportedJSONComparison(ctx, args)
argTp = types.ETString
tp = types.ETString
}
argTps := make([]types.EvalType, len(args))
for i := range args {
argTps[i] = tp
argTps[i] = argTp
}
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, tp, argTps...)
if err != nil {
return nil, err
}
switch tp {
switch argTp {
case types.ETInt:
// adjust unsigned flag
greastInitUnsignedFlag := false
Expand All @@ -495,8 +530,11 @@ func (c *greatestFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
sig = &builtinGreatestDecimalSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_GreatestDecimal)
case types.ETString:
if cmpStringAsDatetime {
sig = &builtinGreatestCmpStringAsTimeSig{bf}
if cmpStringMode == GLCmpStringAsDate {
sig = &builtinGreatestCmpStringAsTimeSig{bf, true}
sig.setPbCode(tipb.ScalarFuncSig_GreatestCmpStringAsDate)
} else if cmpStringMode == GLCmpStringAsDatetime {
sig = &builtinGreatestCmpStringAsTimeSig{bf, false}
sig.setPbCode(tipb.ScalarFuncSig_GreatestCmpStringAsTime)
} else {
sig = &builtinGreatestStringSig{bf}
Expand All @@ -506,8 +544,13 @@ func (c *greatestFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
sig = &builtinGreatestDurationSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_GreatestDuration)
case types.ETDatetime, types.ETTimestamp:
sig = &builtinGreatestTimeSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_GreatestTime)
if fieldTimeType == GLRetDate {
sig = &builtinGreatestTimeSig{bf, true}
sig.setPbCode(tipb.ScalarFuncSig_GreatestDate)
} else {
sig = &builtinGreatestTimeSig{bf, false}
sig.setPbCode(tipb.ScalarFuncSig_GreatestTime)
}
}
sig.getRetTp().Flen, sig.getRetTp().Decimal = fixFlenAndDecimalForGreatestAndLeast(args)
return sig, nil
Expand Down Expand Up @@ -648,11 +691,13 @@ func (b *builtinGreatestStringSig) evalString(row chunk.Row) (max string, isNull

type builtinGreatestCmpStringAsTimeSig struct {
baseBuiltinFunc
cmpAsDate bool
}

func (b *builtinGreatestCmpStringAsTimeSig) Clone() builtinFunc {
newSig := &builtinGreatestCmpStringAsTimeSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
newSig.cmpAsDate = b.cmpAsDate
return newSig
}

Expand All @@ -665,13 +710,9 @@ func (b *builtinGreatestCmpStringAsTimeSig) evalString(row chunk.Row) (strRes st
if isNull || err != nil {
return "", true, err
}
t, err := types.ParseDatetime(sc, v)
v, err = doTimeConversionForGL(b.cmpAsDate, b.ctx, sc, v)
if err != nil {
if err = handleInvalidTimeError(b.ctx, err); err != nil {
return v, true, err
}
} else {
v = t.String()
return v, true, err
}
// In MySQL, if the compare result is zero, than we will try to use the string comparison result
if i == 0 || strings.Compare(v, strRes) > 0 {
Expand All @@ -681,13 +722,39 @@ func (b *builtinGreatestCmpStringAsTimeSig) evalString(row chunk.Row) (strRes st
return strRes, false, nil
}

func doTimeConversionForGL(cmpAsDate bool, ctx sessionctx.Context, sc *stmtctx.StatementContext, strVal string) (string, error) {
var t types.Time
var err error
if cmpAsDate {
t, err = types.ParseDate(sc, strVal)
if err == nil {
t, err = t.Convert(sc, mysql.TypeDate)
}
} else {
t, err = types.ParseDatetime(sc, strVal)
if err == nil {
t, err = t.Convert(sc, mysql.TypeDatetime)
}
}
if err != nil {
if err = handleInvalidTimeError(ctx, err); err != nil {
return "", err
}
} else {
strVal = t.String()
}
return strVal, nil
}

type builtinGreatestTimeSig struct {
baseBuiltinFunc
cmpAsDate bool
}

func (b *builtinGreatestTimeSig) Clone() builtinFunc {
newSig := &builtinGreatestTimeSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
newSig.cmpAsDate = b.cmpAsDate
return newSig
}

Expand All @@ -701,6 +768,12 @@ func (b *builtinGreatestTimeSig) evalTime(row chunk.Row) (res types.Time, isNull
res = v
}
}
// Convert ETType Time value to MySQL actual type, distinguish date and datetime
sc := b.ctx.GetSessionVars().StmtCtx
resTimeTp := getAccurateTimeTypeForGLRet(b.cmpAsDate)
if res, err = res.Convert(sc, resTimeTp); err != nil {
return types.ZeroTime, true, handleInvalidTimeError(b.ctx, err)
}
return res, false, nil
}

Expand Down Expand Up @@ -735,17 +808,19 @@ func (c *leastFunctionClass) getFunction(ctx sessionctx.Context, args []Expressi
if err = c.verifyArgs(args); err != nil {
return nil, err
}
tp, cmpStringAsDatetime := resolveType4Extremum(args)
if cmpStringAsDatetime {
tp, fieldTimeType, cmpStringMode := resolveType4Extremum(args)
argTp := tp
if cmpStringMode != GLCmpStringDirectly {
// Args are temporal and string mixed, we cast all args as string and parse it to temporal mannualy to compare.
tp = types.ETString
argTp = types.ETString
} else if tp == types.ETJson {
unsupportedJSONComparison(ctx, args)
argTp = types.ETString
tp = types.ETString
}
argTps := make([]types.EvalType, len(args))
for i := range args {
argTps[i] = tp
argTps[i] = argTp
}
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, tp, argTps...)
if err != nil {
Expand All @@ -770,8 +845,11 @@ func (c *leastFunctionClass) getFunction(ctx sessionctx.Context, args []Expressi
sig = &builtinLeastDecimalSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_LeastDecimal)
case types.ETString:
if cmpStringAsDatetime {
sig = &builtinLeastCmpStringAsTimeSig{bf}
if cmpStringMode == GLCmpStringAsDate {
sig = &builtinLeastCmpStringAsTimeSig{bf, true}
sig.setPbCode(tipb.ScalarFuncSig_LeastCmpStringAsDate)
} else if cmpStringMode == GLCmpStringAsDatetime {
sig = &builtinLeastCmpStringAsTimeSig{bf, false}
sig.setPbCode(tipb.ScalarFuncSig_LeastCmpStringAsTime)
} else {
sig = &builtinLeastStringSig{bf}
Expand All @@ -781,8 +859,13 @@ func (c *leastFunctionClass) getFunction(ctx sessionctx.Context, args []Expressi
sig = &builtinLeastDurationSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_LeastDuration)
case types.ETDatetime, types.ETTimestamp:
sig = &builtinLeastTimeSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_LeastTime)
if fieldTimeType == GLRetDate {
sig = &builtinLeastTimeSig{bf, true}
sig.setPbCode(tipb.ScalarFuncSig_LeastDate)
} else {
sig = &builtinLeastTimeSig{bf, false}
sig.setPbCode(tipb.ScalarFuncSig_LeastTime)
}
}
sig.getRetTp().Flen, sig.getRetTp().Decimal = fixFlenAndDecimalForGreatestAndLeast(args)
return sig, nil
Expand Down Expand Up @@ -910,11 +993,13 @@ func (b *builtinLeastStringSig) evalString(row chunk.Row) (min string, isNull bo

type builtinLeastCmpStringAsTimeSig struct {
baseBuiltinFunc
cmpAsDate bool
}

func (b *builtinLeastCmpStringAsTimeSig) Clone() builtinFunc {
newSig := &builtinLeastCmpStringAsTimeSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
newSig.cmpAsDate = b.cmpAsDate
return newSig
}

Expand All @@ -927,13 +1012,9 @@ func (b *builtinLeastCmpStringAsTimeSig) evalString(row chunk.Row) (strRes strin
if isNull || err != nil {
return "", true, err
}
t, err := types.ParseDatetime(sc, v)
v, err = doTimeConversionForGL(b.cmpAsDate, b.ctx, sc, v)
if err != nil {
if err = handleInvalidTimeError(b.ctx, err); err != nil {
return v, true, err
}
} else {
v = t.String()
return v, true, err
}
if i == 0 || strings.Compare(v, strRes) < 0 {
strRes = v
Expand All @@ -945,11 +1026,13 @@ func (b *builtinLeastCmpStringAsTimeSig) evalString(row chunk.Row) (strRes strin

type builtinLeastTimeSig struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vecEvalxxx of the sig also needs to be fixed.

baseBuiltinFunc
cmpAsDate bool
}

func (b *builtinLeastTimeSig) Clone() builtinFunc {
newSig := &builtinLeastTimeSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
newSig.cmpAsDate = b.cmpAsDate
return newSig
}

Expand All @@ -963,9 +1046,25 @@ func (b *builtinLeastTimeSig) evalTime(row chunk.Row) (res types.Time, isNull bo
res = v
}
}
// Convert ETType Time value to MySQL actual type, distinguish date and datetime
sc := b.ctx.GetSessionVars().StmtCtx
resTimeTp := getAccurateTimeTypeForGLRet(b.cmpAsDate)
if res, err = res.Convert(sc, resTimeTp); err != nil {
return types.ZeroTime, true, handleInvalidTimeError(b.ctx, err)
}
return res, false, nil
}

func getAccurateTimeTypeForGLRet(cmpAsDate bool) byte {
var resTimeTp byte
if cmpAsDate {
resTimeTp = mysql.TypeDate
guo-shaoge marked this conversation as resolved.
Show resolved Hide resolved
} else {
resTimeTp = mysql.TypeDatetime
}
return resTimeTp
}

type builtinLeastDurationSig struct {
baseBuiltinFunc
}
Expand Down
4 changes: 4 additions & 0 deletions expression/builtin_compare_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ func TestGreatestLeastFunc(t *testing.T) {
[]interface{}{905969664.0, 4556, "1990-06-16 17:22:56.005534"},
"905969664", "1990-06-16 17:22:56.005534", false, false,
},
{
[]interface{}{105969664.0, 120000, types.Duration{Duration: 20*time.Hour + 0*time.Minute + 0*time.Second}},
"20:00:00", "105969664", false, false,
},
} {
f0, err := newFunctionForTest(ctx, ast.Greatest, primitiveValsToConstants(ctx, test.args)...)
require.NoError(t, err)
Expand Down
Loading