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

util/types,expression: fix parseDatetime for 11111111111 and 20170118.999 #4273

Merged
merged 11 commits into from Sep 21, 2017

Conversation

Projects
None yet
7 participants
@tiancaiamao
Contributor

tiancaiamao commented Aug 21, 2017

Remove a TODO in the testing code.
PTAL @winkyao

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish

breeswish Aug 21, 2017

Member

Seems that parseDatetime is actually parsing the datetime in decimal format. In MySQL, when 20170118.999 is given as a string, it is an invalid date.

mysql> select date_format('20170118.999', '%Y-%m-%d %H:%i:%s.%f');
+-----------------------------------------------------+
| date_format('20170118.999', '%Y-%m-%d %H:%i:%s.%f') |
+-----------------------------------------------------+
| NULL                                                |
+-----------------------------------------------------+
1 row in set, 1 warning (0.01 sec)

mysql> select date_format(20170118.999, '%Y-%m-%d %H:%i:%s.%f');
+---------------------------------------------------+
| date_format(20170118.999, '%Y-%m-%d %H:%i:%s.%f') |
+---------------------------------------------------+
| 2017-01-18 00:00:00.000000                        |
+---------------------------------------------------+
1 row in set, 1 warning (0.00 sec)

See #4232

Member

breeswish commented Aug 21, 2017

Seems that parseDatetime is actually parsing the datetime in decimal format. In MySQL, when 20170118.999 is given as a string, it is an invalid date.

mysql> select date_format('20170118.999', '%Y-%m-%d %H:%i:%s.%f');
+-----------------------------------------------------+
| date_format('20170118.999', '%Y-%m-%d %H:%i:%s.%f') |
+-----------------------------------------------------+
| NULL                                                |
+-----------------------------------------------------+
1 row in set, 1 warning (0.01 sec)

mysql> select date_format(20170118.999, '%Y-%m-%d %H:%i:%s.%f');
+---------------------------------------------------+
| date_format(20170118.999, '%Y-%m-%d %H:%i:%s.%f') |
+---------------------------------------------------+
| 2017-01-18 00:00:00.000000                        |
+---------------------------------------------------+
1 row in set, 1 warning (0.00 sec)

See #4232

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Aug 21, 2017

Contributor

Get it! @breeswish

Contributor

tiancaiamao commented Aug 21, 2017

Get it! @breeswish

@winkyao

This comment has been minimized.

Show comment
Hide comment
@winkyao

winkyao Aug 30, 2017

Member

@tiancaiamao any update?

Member

winkyao commented Aug 30, 2017

@tiancaiamao any update?

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Sep 14, 2017

Contributor

Still DNM? @tiancaiamao

Contributor

XuHuaiyu commented Sep 14, 2017

Still DNM? @tiancaiamao

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Sep 14, 2017

Contributor

I'll working on it. @XuHuaiyu

Contributor

tiancaiamao commented Sep 14, 2017

I'll working on it. @XuHuaiyu

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Sep 14, 2017

Contributor

@breeswish I find that mysql's behavior when parsing string to datetime is, taking float part as HH:MM:SS

mysql> select date_format('20170118.123423', '%Y-%m-%d %H:%i:%s.%f');
+--------------------------------------------------------+
| date_format('20170118.123423', '%Y-%m-%d %H:%i:%s.%f') |
+--------------------------------------------------------+
| 2017-01-18 12:34:23.000000                             |
+--------------------------------------------------------+
1 row in set (0.00 sec)

'20170118.999' fail to parse because 99:90:00 out of HH:MM:SS range.

Contributor

tiancaiamao commented Sep 14, 2017

@breeswish I find that mysql's behavior when parsing string to datetime is, taking float part as HH:MM:SS

mysql> select date_format('20170118.123423', '%Y-%m-%d %H:%i:%s.%f');
+--------------------------------------------------------+
| date_format('20170118.123423', '%Y-%m-%d %H:%i:%s.%f') |
+--------------------------------------------------------+
| 2017-01-18 12:34:23.000000                             |
+--------------------------------------------------------+
1 row in set (0.00 sec)

'20170118.999' fail to parse because 99:90:00 out of HH:MM:SS range.

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Sep 14, 2017

Contributor

I've update it. Timestamp builtin function (maybe along with others) still have some pitfalls, but it's improving.

I suggest having separate functions for ParseTimeFromString ParseTimeFromFloat ParseTimeFromNum in the future, as their behavior is not the same.

For example,

ParseTimeFromInt(11111111111)  => 2001-11-11 11:11:11
ParseTimeFromString('11111111111') => 2011-11-11 11:11:01

ParseTimeFromFloat(20170118.123) => 2017-01-18 12:03:00
ParseTimeFromString('20170118.123') => 2017-01-18 00:00:00

Convert int or float to string, then string to datetime will get wrong result.

PTAL @XuHuaiyu @breeswish

Contributor

tiancaiamao commented Sep 14, 2017

I've update it. Timestamp builtin function (maybe along with others) still have some pitfalls, but it's improving.

I suggest having separate functions for ParseTimeFromString ParseTimeFromFloat ParseTimeFromNum in the future, as their behavior is not the same.

For example,

ParseTimeFromInt(11111111111)  => 2001-11-11 11:11:11
ParseTimeFromString('11111111111') => 2011-11-11 11:11:01

ParseTimeFromFloat(20170118.123) => 2017-01-18 12:03:00
ParseTimeFromString('20170118.123') => 2017-01-18 00:00:00

Convert int or float to string, then string to datetime will get wrong result.

PTAL @XuHuaiyu @breeswish

@tiancaiamao tiancaiamao removed the status/DNM label Sep 15, 2017

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Sep 20, 2017

Contributor

PTAL @breeswish

Contributor

XuHuaiyu commented Sep 20, 2017

PTAL @breeswish

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish

breeswish Sep 20, 2017

Member

/run-all-test

Member

breeswish commented Sep 20, 2017

/run-all-test

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Sep 21, 2017

Contributor

/run-all-test

Contributor

tiancaiamao commented Sep 21, 2017

/run-all-test

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Sep 21, 2017

PTAL @breeswish

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish

breeswish Sep 21, 2017

Member

LGTM

Member

breeswish commented Sep 21, 2017

LGTM

@breeswish breeswish added status/LGT2 and removed status/LGT1 labels Sep 21, 2017

@coocood coocood merged commit 376a7dd into master Sep 21, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@coocood coocood deleted the tiancaiamao/parse-datetime branch Sep 21, 2017

@mccxj

This comment has been minimized.

Show comment
Hide comment
@mccxj

mccxj Sep 21, 2017

Contributor

Looks still a problem :)

tidb

tidb>select cast(20170118.999 as datetime);
ERROR 1105 (HY000): invalid time format
tidb>select cast('20170118.999' as datetime);
ERROR 1105 (HY000): invalid time format

mysql

mysql> select cast(20170118.999 as datetime);
+--------------------------------+
| cast(20170118.999 as datetime) |
+--------------------------------+
| 2017-01-18 00:00:00            |
+--------------------------------+
1 row in set (0.00 sec)

mysql> select cast('20170118.999' as datetime);
+----------------------------------+
| cast('20170118.999' as datetime) |
+----------------------------------+
| NULL                             |
+----------------------------------+
1 row in set, 1 warning (0.00 sec)
Contributor

mccxj commented Sep 21, 2017

Looks still a problem :)

tidb

tidb>select cast(20170118.999 as datetime);
ERROR 1105 (HY000): invalid time format
tidb>select cast('20170118.999' as datetime);
ERROR 1105 (HY000): invalid time format

mysql

mysql> select cast(20170118.999 as datetime);
+--------------------------------+
| cast(20170118.999 as datetime) |
+--------------------------------+
| 2017-01-18 00:00:00            |
+--------------------------------+
1 row in set (0.00 sec)

mysql> select cast('20170118.999' as datetime);
+----------------------------------+
| cast('20170118.999' as datetime) |
+----------------------------------+
| NULL                             |
+----------------------------------+
1 row in set, 1 warning (0.00 sec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment