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.

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

parser, ast, expression: fix bug on DATE literal #4362

Merged
merged 21 commits into from
Sep 15, 2017
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
33a0aaf
parser: add 'TIME', 'TIMESTAMP' literal
spongedu Aug 29, 2017
f9aef71
parser, expression: fix bug on DATE literal
spongedu Aug 29, 2017
95adc56
add illegal time format tests in integration_test.go
spongedu Aug 29, 2017
e21a583
remove unneccessary modifications
spongedu Aug 29, 2017
581abaf
bug fix
spongedu Aug 29, 2017
d1f5df9
resolve conflicts
spongedu Aug 30, 2017
09c6f08
Merge branch 'master' into issue_4307
spongedu Aug 30, 2017
503db45
Merge remote-tracking branch 'upstream/master' into issue_4307
spongedu Aug 31, 2017
db0ad3d
1. evaluate date literal before execution 2. add tests in typeinfer_t…
spongedu Aug 31, 2017
552bf49
Merge branch 'issue_4307' of github.com:spongedu/tidb into issue_4307
spongedu Aug 31, 2017
49a945f
code refine
spongedu Aug 31, 2017
1d62af9
Merge remote-tracking branch 'upstream/master' into issue_4307
spongedu Sep 9, 2017
b0f844b
Merge branch 'master' into issue_4307
spongedu Sep 11, 2017
02248c7
code refine
spongedu Sep 13, 2017
9a26d31
Merge remote-tracking branch 'upstream/master' into issue_4307
spongedu Sep 13, 2017
ff2b23e
Merge branch 'issue_4307' of github.com:spongedu/tidb into issue_4307
spongedu Sep 13, 2017
6c8296b
1. teake sql_mode into consideration 2. add more tests related to sql…
spongedu Sep 14, 2017
e1f107b
Merge branch 'master' into issue_4307
spongedu Sep 14, 2017
2f430ae
code refine
spongedu Sep 14, 2017
1065b7f
1. code refine 2. add tests for SQLMode
spongedu Sep 14, 2017
1c1ea57
Merge remote-tracking branch 'upstream/master' into issue_4307
spongedu Sep 14, 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
1 change: 1 addition & 0 deletions ast/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ const (
CurrentTimestamp = "current_timestamp"
Curtime = "curtime"
Date = "date"
DateLiteral = "dateliteral"
DateAdd = "date_add"
DateFormat = "date_format"
DateSub = "date_sub"
Expand Down
1 change: 1 addition & 0 deletions expression/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ var funcs = map[string]functionClass{
ast.CurrentTimestamp: &nowFunctionClass{baseFunctionClass{ast.CurrentTimestamp, 0, 1}},
ast.Curtime: &currentTimeFunctionClass{baseFunctionClass{ast.Curtime, 0, 1}},
ast.Date: &dateFunctionClass{baseFunctionClass{ast.Date, 1, 1}},
ast.DateLiteral: &dateLiteralFunctionClass{baseFunctionClass{ast.DateLiteral, 1, 1}},
ast.DateAdd: &dateArithFunctionClass{baseFunctionClass{ast.DateAdd, 3, 3}, ast.DateArithAdd},
ast.DateFormat: &dateFormatFunctionClass{baseFunctionClass{ast.DateFormat, 2, 2}},
ast.DateSub: &dateArithFunctionClass{baseFunctionClass{ast.DateSub, 3, 3}, ast.DateArithSub},
Expand Down
57 changes: 57 additions & 0 deletions expression/builtin_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ const ( // GET_FORMAT location.
// DurationPattern determine whether to match the format of duration.
var DurationPattern = regexp.MustCompile(`^(|[-]?)(|\d{1,2}\s)(\d{2,3}:\d{2}:\d{2}|\d{1,2}:\d{2}|\d{1,6})(|\.\d*)$`)

// DatePattern determine whether to match the format of date.
var DatePattern = regexp.MustCompile(`^\s*((0*\d{1,4}([^\d]0*\d{1,2}){2})|(\d{2,4}(\d{2}){2}))\s*$`)
Copy link
Member

Choose a reason for hiding this comment

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

no need to export this variable, so does DurationPattern

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


var (
_ functionClass = &dateFunctionClass{}
_ functionClass = &dateLiteralFunctionClass{}
_ functionClass = &dateDiffFunctionClass{}
_ functionClass = &timeDiffFunctionClass{}
_ functionClass = &dateFormatFunctionClass{}
Expand Down Expand Up @@ -107,6 +111,7 @@ var (

var (
_ builtinFunc = &builtinDateSig{}
_ builtinFunc = &builtinDateLiteralSig{}
_ builtinFunc = &builtinDateDiffSig{}
_ builtinFunc = &builtinTimeDiffSig{}
_ builtinFunc = &builtinDateFormatSig{}
Expand Down Expand Up @@ -283,6 +288,58 @@ func (b *builtinDateSig) evalTime(row []types.Datum) (types.Time, bool, error) {
return expr, false, nil
}

type dateLiteralFunctionClass struct {
baseFunctionClass
}

func (c *dateLiteralFunctionClass) getFunction(ctx context.Context, args []Expression) (builtinFunc, error) {
if err := c.verifyArgs(args); err != nil {
return nil, errors.Trace(err)
}
constant, ok := args[0].(*Constant)
if !ok {
return nil, errors.Trace(types.ErrInvalidTimeFormat)
Copy link
Member

Choose a reason for hiding this comment

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

should we handle this error to respect to sql mode ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope.

}
str := constant.Value.GetString()
if !DatePattern.MatchString(str) {
return nil, errors.Trace(types.ErrInvalidTimeFormat)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

}
tm, err := types.ParseDate(str)
if err != nil {
return nil, errors.Trace(err)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

}
bf := newBaseBuiltinFuncWithTp(args, ctx, tpDatetime, tpString)
bf.tp.Tp, bf.tp.Flen, bf.tp.Decimal = mysql.TypeDate, 10, 0
sig := &builtinDateLiteralSig{baseTimeBuiltinFunc{bf}, tm}
return sig.setSelf(sig), nil
}

type builtinDateLiteralSig struct {
baseTimeBuiltinFunc
tm types.Time
Copy link
Member

Choose a reason for hiding this comment

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

s/tm/literal/ ?

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

}

// evalTime evals DATE 'stringLit'.
// See https://dev.mysql.com/doc/refman/5.7/en/date-and-time-literals.html
func (b *builtinDateLiteralSig) evalTime(row []types.Datum) (types.Time, bool, error) {
/*
sc := b.ctx.GetSessionVars().StmtCtx
str, isNull, err := b.args[0].EvalString(row, sc)
if isNull || err != nil {
return types.Time{}, true, errors.Trace(err)
}
if !DatePattern.MatchString(str) {
return types.Time{}, true, errors.Trace(types.ErrInvalidTimeFormat)
}
ret, err := types.ParseDate(str)
if err != nil {
return types.Time{}, true, errors.Trace(err)
}
return ret, false, nil
*/
Copy link
Member

Choose a reason for hiding this comment

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

remove the commented code

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

return b.tm, false, nil
}

func convertDatumToTime(sc *variable.StatementContext, d types.Datum) (t types.Time, err error) {
if d.Kind() != types.KindMysqlTime {
d, err = convertToTime(sc, d, mysql.TypeDatetime)
Expand Down
33 changes: 23 additions & 10 deletions expression/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2422,21 +2422,34 @@ func (s *testIntegrationSuite) TestDateBuiltin(c *C) {
r = tk.MustQuery("select date'2017/12-12'")
r.Check(testkit.Rows("2017-12-12"))

r = tk.MustQuery("select date'0000-00-00'")
r.Check(testkit.Rows("<nil>"))
r = tk.MustQuery("select date '0000-00-00';")
r.Check(testkit.Rows("0000-00-00"))
Copy link
Contributor

Choose a reason for hiding this comment

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

in mysql, this will cause an error.

Copy link
Contributor Author

@spongedu spongedu Sep 14, 2017

Choose a reason for hiding this comment

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

@XuHuaiyu What's your mysql version? It works on my computer.

# ./mysql -uroot 
mysql: [Warning] Using a password on the command line interface can be insecure.
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 35
Server version: 5.7.17-log MySQL Community Server (GPL)

Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.

Oracle is a registered trademark of Oracle Corporation and/or its
affiliates. Other names may be trademarks of their respective
owners.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> select date '0000-00-00';
+-------------------+
| date '0000-00-00' |
+-------------------+
| 0000-00-00        |
+-------------------+
1 row in set (0.00 sec)

mysql> 

Copy link
Contributor Author

@spongedu spongedu Sep 14, 2017

Choose a reason for hiding this comment

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

@XuHuaiyu The result seems has something to do with SQL_MODE. if NO_ZERO_DATE is set in SQL_MODE 0000-00-00 is not allowed. Otherwise it works. I'll fix this.


r = tk.MustQuery("select date'0000-00-00 00:00:00'")
r.Check(testkit.Rows("<nil>"))
r = tk.MustQuery("select date'1998~01~02'")
r.Check(testkit.Rows("1998-01-02"))

r = tk.MustQuery("select date'2017-99-99';")
r.Check(testkit.Rows("<nil>"))
r = tk.MustQuery("select date'731124', date '011124'")
r.Check(testkit.Rows("1973-11-24 2001-11-24"))
Copy link
Member

Choose a reason for hiding this comment

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

add test for invalid time format

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


r = tk.MustQuery("select date'2017-2-31';")
r.Check(testkit.Rows("<nil>"))
_, err := tk.Exec("select date '0000-00-00 00:00:00';")
c.Assert(err, NotNil)
Copy link
Member

Choose a reason for hiding this comment

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

how about add one more check for error message ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done @zz-jason

c.Assert(terror.ErrorEqual(err, types.ErrInvalidTimeFormat), IsTrue)

_, err = tk.Exec("select date '2017-99-99';")
c.Assert(err, NotNil)
c.Assert(terror.ErrorEqual(err, types.ErrInvalidTimeFormat), IsTrue)

_, err = tk.Exec("select date '2017-2-31';")
c.Assert(err, NotNil)
c.Assert(terror.ErrorEqual(err, types.ErrInvalidTimeFormat), IsTrue)

r = tk.MustQuery("select date'201712-31';")
r.Check(testkit.Rows("<nil>"))
_, err = tk.Exec("select date '201712-31';")
c.Assert(err, NotNil)
c.Assert(terror.ErrorEqual(err, types.ErrInvalidTimeFormat), IsTrue)

_, err = tk.Exec("select date 'abcdefg';")
c.Assert(err, NotNil)
c.Assert(terror.ErrorEqual(err, types.ErrInvalidTimeFormat), IsTrue)
}

func (s *testIntegrationSuite) TestLiterals(c *C) {
Expand Down
2 changes: 1 addition & 1 deletion parser/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,7 @@ FunctionCallConflict:
| "DATE" stringLit
{
expr := ast.NewValueExpr($2)
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr(ast.Date), Args: []ast.ExprNode{expr}}
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr(ast.DateLiteral), Args: []ast.ExprNode{expr}}
}

DistinctKwd:
Expand Down
6 changes: 6 additions & 0 deletions plan/typeinfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,12 @@ func (s *testPlanSuite) createTestCase4LikeFuncs() []typeInferTestCase {
}
}

func (s *testPlanSuite) createTestCase4Literals() []typeInferTestCase {
return []typeInferTestCase{
{"date '2017-01-01'", mysql.TypeDate, charset.CharsetBin, mysql.BinaryFlag, 0, 10},
}
}

func (s *testPlanSuite) createTestCase4JSONFuncs() []typeInferTestCase {
return []typeInferTestCase{
{"json_type(c_json)", mysql.TypeVarString, charset.CharsetUTF8, 0, 51, types.UnspecifiedLength},
Expand Down