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

evaluator, parser: support UTC_DATE() #1045

Merged
merged 2 commits into from
Apr 5, 2016

Conversation

overvenus
Copy link
Member

see issue: #236.

@shenli
Copy link
Member

shenli commented Apr 1, 2016

@overvenus Thanks for your report! We will add more builtin functions.
If you are interested in TiDB, would you please send us a PR? We have a tutorial about how to add a new builtin function. We will be appreciated.

@overvenus
Copy link
Member Author

@shenli Hi, and yes, I have just added a new builtin function, it is a PR not issue. Maybe I should change title to Add UTC_DATE()?

@shenli
Copy link
Member

shenli commented Apr 1, 2016

Sorry.... It is my mistake.
Great! I will take a look!

@@ -316,6 +316,14 @@ func (s *testEvaluatorSuite) TestCurrentTime(c *C) {
c.Assert(err, NotNil)
}

func (s *testEvaluatorSuite) TestUTCDate(c *C) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add another case like:
mysql> select utc_date + 1;
+--------------+
| utc_date + 1 |
+--------------+
| 20160402 |
+--------------+

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I will take a look, and maybe modification of current_date() is also needed.

@shenli
Copy link
Member

shenli commented Apr 1, 2016

@coocood @qiuyesuifeng PTAL

@shenli
Copy link
Member

shenli commented Apr 4, 2016

@overvenus Any update for this PR?

@overvenus
Copy link
Member Author

@shenli @qiuyesuifeng PTAL

@shenli
Copy link
Member

shenli commented Apr 4, 2016

LGTM
@coocood @qiuyesuifeng

@qiuyesuifeng
Copy link
Member

@overvenus Thanks.
LGTM

@overvenus
Copy link
Member Author

Squashed. @shenli @qiuyesuifeng

cc @c4pt0r

@@ -390,6 +390,16 @@ func builtinCurrentTime(args []types.Datum, _ context.Context) (d types.Datum, e
return convertToDuration(d, fsp)
}

// See https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_utc-date
func builtinUTCDate(args []types.Datum, _ context.Context) (d types.Datum, err error) {
year, month, day := time.Now().Date()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should change this to time.Now().UTC().Date()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I'm sorry, you are right, thank you for pointing out, I will fix it tomorrow.

@overvenus
Copy link
Member Author

PTAL

@shenli
Copy link
Member

shenli commented Apr 5, 2016

LGTM
Thanks! @overvenus

@shenli shenli merged commit d01d516 into pingcap:master Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants