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

types,expression: Fix parse time from string #7654

Merged
merged 7 commits into from
Sep 11, 2018
Merged

Conversation

spongedu
Copy link
Contributor

What problem does this PR solve?

What is changed and how it works?

Currently TiDB fail to deal with some cases when parse string to datetime, This pr fixes these cases. For example, in MySQL:

mysql> select cast("1701020301" as datetime);
+--------------------------------+
| cast("1701020301" as datetime) |
+--------------------------------+
| 2017-01-02 03:01:00            |
+--------------------------------+
1 row in set (0.00 sec)

mysql> select cast("170102034" as datetime);
+-------------------------------+
| cast("170102034" as datetime) |
+-------------------------------+
| 2017-01-02 03:04:00           |
+-------------------------------+
1 row in set (0.00 sec)

While in TiDB:

tidb> select cast("1701020301." as datetime);
ERROR 1105 (HY000): invalid time format
tidb> select cast("1701020301" as datetime);
ERROR 1105 (HY000): invalid time format
tidb> select cast("170102034" as datetime);
ERROR 1105 (HY000): invalid time format

Check List

Tests

  • Unit test
  • Integration test

types/time.go Outdated
_, err = fmt.Sscanf(fracStr, "%1d", &second)
default:
_, err = fmt.Sscanf(fracStr[:2], "%2d", &second)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should we raise warning as MySQL does for the case below?

mysql> select cast("170102037.1122" as datetime);
+------------------------------------+
| cast("170102037.1122" as datetime) |
+------------------------------------+
| 2017-01-02 03:07:11                |
+------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+------------------------------------------------------+
| Level   | Code | Message                                              |
+---------+------+------------------------------------------------------+
| Warning | 1292 | Truncated incorrect datetime value: '170102037.1122' |
+---------+------+------------------------------------------------------+
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.

Yes, indeed. I'll fix here

@zz-jason
Copy link
Member

@spongedu We use go 1.11 now, maybe you can update go to make fmt happy.

@spongedu
Copy link
Contributor Author

@zz-jason ok

@spongedu
Copy link
Contributor Author

@zz-jason done here. PTAL

@eurekaka
Copy link
Contributor

/run-all-tests

Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 11, 2018
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 11, 2018
@coocood coocood merged commit 5ad005b into pingcap:master Sep 11, 2018
@XuHuaiyu XuHuaiyu added the type/bugfix This PR fixes a bug. label Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/mysql-protocol contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants