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

Jump to bottom

expression: rewrite builtin function MAKETIME #4396

Merged
merged 11 commits into from
Sep 13, 2017
Merged

expression: rewrite builtin function MAKETIME #4396

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f6d2aa4
expression: rewrite builtin function MAKETIME
spongedu Aug 31, 2017
06aec63
resolve merge conflicts
spongedu Aug 31, 2017
38a95e2
Merge branch 'master' into rewrite_maketime
zz-jason Sep 2, 2017
398c749
1. resolve merge conflicts 2. use 'evelTp' to replace typeClass in ma…
spongedu Sep 9, 2017
edfd0ec
Merge branch 'rewrite_maketime' of github.com:spongedu/tidb into rewr…
spongedu Sep 9, 2017
b4a91d2
code refine
spongedu Sep 9, 2017
655d7b7
Merge branch 'master' into rewrite_maketime
zz-jason Sep 11, 2017
ab01731
Merge remote-tracking branch 'upstream/master' into rewrite_maketime
spongedu Sep 11, 2017
e0140a3
code refine
spongedu Sep 11, 2017
b0016dd
Merge branch 'rewrite_maketime' of github.com:spongedu/tidb into rewr…
spongedu Sep 11, 2017
4544ae4
Merge branch 'master' into rewrite_maketime
XuHuaiyu Sep 13, 2017
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
86 changes: 42 additions & 44 deletions expression/builtin_time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2658,34 +2658,54 @@ func (c *makeTimeFunctionClass) getFunction(ctx context.Context, args []Expressi
if err := c.verifyArgs(args); err != nil {
return nil, errors.Trace(err)
}
sig := &builtinMakeTimeSig{newBaseBuiltinFunc(args, ctx)}
evelTp, flen, decimal := fieldTp2EvalTp(args[2].GetType()), 10, 0
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ evelTp/ evalTp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@XuHuaiyu Done

switch evelTp {
case tpInt:
case tpReal, tpDecimal:
decimal = args[2].GetType().Decimal
if decimal > 6 || decimal == types.UnspecifiedLength {
decimal = 6
}
if decimal > 0 {
flen += 1 + decimal
}
default:
flen, decimal = 17, 6
}
bf := newBaseBuiltinFuncWithTp(args, ctx, tpDuration, tpInt, tpInt, tpReal)
bf.tp.Flen, bf.tp.Decimal = flen, decimal
sig := &builtinMakeTimeSig{baseDurationBuiltinFunc{bf}}
return sig.setSelf(sig), nil
}

type builtinMakeTimeSig struct {
baseBuiltinFunc
baseDurationBuiltinFunc
}

// eval evals a builtinMakeTimeSig.
// evalDuration evals a builtinMakeTimeIntSig.
// See https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_maketime
func (b *builtinMakeTimeSig) eval(row []types.Datum) (d types.Datum, err error) {
args, err := b.evalArgs(row)
if err != nil {
return d, errors.Trace(err)
func (b *builtinMakeTimeSig) evalDuration(row []types.Datum) (types.Duration, bool, error) {
sc, dur := b.ctx.GetSessionVars().StmtCtx, types.ZeroDuration
dur.Fsp = types.MaxFsp
hour, isNull, err := b.args[0].EvalInt(row, sc)
if isNull || err != nil {
return dur, isNull, errors.Trace(handleInvalidTimeError(b.ctx, err))
}
// MAKETIME(hour, minute, second)
if args[0].IsNull() || args[1].IsNull() || args[2].IsNull() {
return
minute, isNull, err := b.args[1].EvalInt(row, sc)
if isNull || err != nil {
return dur, isNull, errors.Trace(handleInvalidTimeError(b.ctx, err))
}

var (
hour int64
minute int64
second float64
overflow bool
)
sc := b.ctx.GetSessionVars().StmtCtx
hour, _ = args[0].ToInt64(sc)
if minute < 0 || minute >= 60 {
return dur, true, nil
}
second, isNull, err := b.args[2].EvalReal(row, sc)
if isNull || err != nil {
return dur, isNull, errors.Trace(err)
}
if second < 0 || second >= 60 {
return dur, true, nil
}
var overflow bool
// MySQL TIME datatype: https://dev.mysql.com/doc/refman/5.7/en/time.html
// ranges from '-838:59:59.000000' to '838:59:59.000000'
if hour < -838 {
Expand All @@ -2695,16 +2715,6 @@ func (b *builtinMakeTimeSig) eval(row []types.Datum) (d types.Datum, err error)
hour = 838
overflow = true
}

minute, _ = args[1].ToInt64(sc)
if minute < 0 || minute >= 60 {
return
}

second, _ = args[2].ToFloat64(sc)
if second < 0 || second >= 60 {
return
}
if hour == -838 || hour == 838 {
if second > 59 {
second = 59
Expand All @@ -2714,24 +2724,12 @@ func (b *builtinMakeTimeSig) eval(row []types.Datum) (d types.Datum, err error)
minute = 59
second = 59
}

var dur types.Duration
fsp := types.MaxFsp
if args[2].Kind() != types.KindString {
sec, _ := args[2].ToString()
secs := strings.Split(sec, ".")
if len(secs) <= 1 {
fsp = 0
} else if len(secs[1]) < fsp {
fsp = len(secs[1])
}
}
fsp := b.self.getRetTp().Decimal
Copy link
Member

Choose a reason for hiding this comment

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

fsp = b.tp.Decimal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

dur, err = types.ParseDuration(fmt.Sprintf("%02d:%02d:%v", hour, minute, second), fsp)
if err != nil {
return d, errors.Trace(err)
return dur, true, errors.Trace(err)
}
d.SetMysqlDuration(dur)
return
return dur, false, nil
}

type periodAddFunctionClass struct {
Expand Down
44 changes: 31 additions & 13 deletions expression/builtin_time_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,37 +1615,35 @@ func (s *testEvaluatorSuite) TestMakeTime(c *C) {
{[]interface{}{12, 15, -30}, nil},

{[]interface{}{12, 15, "30.10"}, "12:15:30.100000"},
{[]interface{}{12, 15, "30.00"}, "12:15:30.000000"},
{[]interface{}{12, 15, 30.0000001}, "12:15:30.000000"},
{[]interface{}{12, 15, "30.20"}, "12:15:30.200000"},
{[]interface{}{12, 15, 30.3000001}, "12:15:30.300000"},
{[]interface{}{12, 15, 30.0000005}, "12:15:30.000001"},
{[]interface{}{"12", "15", 30.1}, "12:15:30.1"},
{[]interface{}{"12", "15", 30.1}, "12:15:30.100000"},

{[]interface{}{0, 58.4, 0}, "00:58:00"},
{[]interface{}{0, "58.4", 0}, "00:58:00"},
{[]interface{}{0, 58.5, 1}, "00:59:01"},
{[]interface{}{0, "58.5", 1}, "00:58:01"},
{[]interface{}{0, 59.5, 1}, nil},
{[]interface{}{0, "59.5", 1}, "00:59:01"},
{[]interface{}{0, 1, 59.1}, "00:01:59.1"},
{[]interface{}{0, 1, 59.1}, "00:01:59.100000"},
{[]interface{}{0, 1, "59.1"}, "00:01:59.100000"},
{[]interface{}{0, 1, 59.5}, "00:01:59.5"},
{[]interface{}{0, 1, 59.5}, "00:01:59.500000"},
{[]interface{}{0, 1, "59.5"}, "00:01:59.500000"},
{[]interface{}{23.5, 1, 10}, "24:01:10"},
{[]interface{}{"23.5", 1, 10}, "23:01:10"},

{[]interface{}{0, 0, 0}, "00:00:00"},
{[]interface{}{"", "", ""}, "00:00:00.000000"},
{[]interface{}{"h", "m", "s"}, "00:00:00.000000"},

{[]interface{}{837, 59, 59.1}, "837:59:59.1"},
{[]interface{}{838, 59, 59.1}, "838:59:59.0"},
{[]interface{}{-838, 59, 59.1}, "-838:59:59.0"},
{[]interface{}{837, 59, 59.1}, "837:59:59.100000"},
Copy link
Member

Choose a reason for hiding this comment

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

in mysql:

MySQL > select maketime(837, 59, 59.1);
-------------------------+
| maketime(837, 59, 59.1) |
+-------------------------+
| 837:59:59.1             |
+-------------------------+
1 row in set (0.00 sec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. You can see my integration tests.There's a similar case and the result is correct. Decimals goes wrong here because 'datumsToConstants' don't correctly set constants's decimal(all are set -1, which is not as expected). So here the tests equal to creating a table with a double column, and use it as the third parameter in MAKETIME

Copy link
Contributor Author

@spongedu spongedu Sep 1, 2017 •
edited by zz-jason
Loading

Choose a reason for hiding this comment

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

@zz-jason

{[]interface{}{838, 59, 59.1}, "838:59:59.000000"},
{[]interface{}{-838, 59, 59.1}, "-838:59:59.000000"},
{[]interface{}{1000, 1, 1}, "838:59:59"},
{[]interface{}{-1000, 1, 1.23}, "-838:59:59.00"},
{[]interface{}{-1000, 1, 1.23}, "-838:59:59.000000"},
{[]interface{}{1000, 59.1, 1}, "838:59:59"},
{[]interface{}{1000, 59.5, 1}, nil},
{[]interface{}{1000, 1, 59.1}, "838:59:59.0"},
{[]interface{}{1000, 1, 59.5}, "838:59:59.0"},
{[]interface{}{1000, 1, 59.1}, "838:59:59.000000"},
{[]interface{}{1000, 1, 59.5}, "838:59:59.000000"},

{[]interface{}{12, 15, 60}, nil},
{[]interface{}{12, 15, "60"}, nil},
Expand All @@ -1663,6 +1661,7 @@ func (s *testEvaluatorSuite) TestMakeTime(c *C) {
for idx, t := range Dtbl {
f, err := maketime.getFunction(s.ctx, datumsToConstants(t["Args"]))
c.Assert(err, IsNil)
c.Assert(f.canBeFolded(), IsTrue)
got, err := f.eval(nil)
c.Assert(err, IsNil)
if t["Want"][0].Kind() == types.KindNull {
Expand All @@ -1673,6 +1672,25 @@ func (s *testEvaluatorSuite) TestMakeTime(c *C) {
c.Assert(got.GetMysqlDuration().String(), Equals, want, Commentf("[%v] - args:%v", idx, t["Args"]))
}
}

tbl = []struct {
Args []interface{}
Want interface{}
}{
{[]interface{}{"", "", ""}, "00:00:00"},
{[]interface{}{"h", "m", "s"}, "00:00:00"},
}
Dtbl = tblToDtbl(tbl)
maketime = funcs[ast.MakeTime]
for idx, t := range Dtbl {
f, err := maketime.getFunction(s.ctx, datumsToConstants(t["Args"]))
c.Assert(err, IsNil)
c.Assert(f.canBeFolded(), IsTrue)
got, err := f.eval(nil)
c.Assert(err, NotNil)
want, err := t["Want"][0].ToString()
c.Assert(got.GetMysqlDuration().String(), Equals, want, Commentf("[%v] - args:%v", idx, t["Args"]))
}
}

func (s *testEvaluatorSuite) TestQuarter(c *C) {
Expand Down
13 changes: 13 additions & 0 deletions expression/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,19 @@ func (s *testIntegrationSuite) TestTimeBuiltin(c *C) {
result.Check(testkit.Rows("0000-00-01 <nil>"))
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1105|invalid time format"))

// for maketime
tk.MustExec(`drop table if exists t`)
tk.MustExec(`create table t(a double, b float, c decimal(10,4));`)
tk.MustExec(`insert into t value(1.23, 2.34, 3.1415)`)
result = tk.MustQuery("select maketime(1,1,a), maketime(2,2,b), maketime(3,3,c) from t;")
result.Check(testkit.Rows("01:01:01.230000 02:02:02.340000 03:03:03.1415"))
result = tk.MustQuery("select maketime(12, 13, 14), maketime('12', '15', 30.1), maketime(0, 1, 59.1), maketime(0, 1, '59.1'), maketime(0, 1, 59.5)")
result.Check(testkit.Rows("12:13:14 12:15:30.1 00:01:59.1 00:01:59.100000 00:01:59.5"))
result = tk.MustQuery("select maketime(12, 15, 60), maketime(12, 15, '60'), maketime(12, 60, 0), maketime(12, 15, null)")
result.Check(testkit.Rows("<nil> <nil> <nil> <nil>"))
result = tk.MustQuery("select maketime('', '', ''), maketime('h', 'm', 's');")
result.Check(testkit.Rows("00:00:00.000000 00:00:00.000000"))

// for get_format
result = tk.MustQuery(`select GET_FORMAT(DATE,'USA'), GET_FORMAT(DATE,'JIS'), GET_FORMAT(DATE,'ISO'), GET_FORMAT(DATE,'EUR'),
GET_FORMAT(DATE,'INTERNAL'), GET_FORMAT(DATETIME,'USA') , GET_FORMAT(DATETIME,'JIS'), GET_FORMAT(DATETIME,'ISO'),
Expand Down
7 changes: 7 additions & 0 deletions plan/typeinfer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,13 @@ func (s *testPlanSuite) createTestCase4TimeFuncs() []typeInferTestCase {
{"period_diff(c_set , c_int_d)", mysql.TypeLonglong, charset.CharsetBin, mysql.BinaryFlag, 6, 0},
{"period_diff(c_enum , c_int_d)", mysql.TypeLonglong, charset.CharsetBin, mysql.BinaryFlag, 6, 0},

{"maketime(c_int_d, c_int_d, c_double_d)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 17, 6},
{"maketime(c_int_d, c_int_d, c_decimal)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 14, 3},
{"maketime(c_int_d, c_int_d, c_decimal_d)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 10, 0},
{"maketime(c_int_d, c_int_d, c_char)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 17, 6},
{"maketime(c_int_d, c_int_d, c_varchar)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 17, 6},
{"maketime(c_int_d, c_int_d, 1.2345)", mysql.TypeDuration, charset.CharsetBin, mysql.BinaryFlag, 15, 4},

{"get_format(DATE, 'USA')", mysql.TypeVarString, charset.CharsetUTF8, 0, 17, types.UnspecifiedLength},
{"from_unixtime(20170101.999)", mysql.TypeDatetime, charset.CharsetBin, mysql.BinaryFlag, mysql.MaxDatetimeWidthWithFsp, 3},
{"from_unixtime(20170101.1234567)", mysql.TypeDatetime, charset.CharsetBin, mysql.BinaryFlag, mysql.MaxDatetimeWidthWithFsp, types.MaxFsp},
Expand Down