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

Support ambiguous time #3786

Merged
merged 5 commits into from Nov 16, 2018

Conversation

@breeswish
Copy link
Member

commented Nov 14, 2018

Signed-off-by: Breezewish breezewish@pingcap.com

What have you changed? (mandatory)

Supports parsing date time which is ambiguous due to DST shifting back.

This PR is a workaround that uses some low level chrono API to bypass chronotope/chrono-tz#23. Once chronotope/chrono-tz#23 is fixed, we should switch back using public and high level APIs.

Resolve #3721

What are the type of the changes? (mandatory)

  • Bug fix (non-breaking change which fixes an issue)

How has this PR been tested? (mandatory)

Unit test.

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

breeswish added 2 commits Nov 14, 2018
Signed-off-by: Breezewish <breezewish@pingcap.com>
@breeswish

This comment has been minimized.

Copy link
Member Author

commented Nov 15, 2018

@BusyJay @solotzg @DorianZheng This is an important bug fix required by our new clients. PTAL, thanks!

@breeswish breeswish added P: High and removed P: Critical labels Nov 15, 2018
.and_then(|t| t.checked_add_signed(Duration::nanoseconds(i64::from(nanos))))
.and_then(|datetime| tz.from_local_datetime(&datetime).earliest())

This comment has been minimized.

Copy link
@BusyJay

BusyJay Nov 15, 2018

Contributor

Why is earliest chosen?

This comment has been minimized.

Copy link
@breeswish

breeswish Nov 15, 2018

Author Member

Because MySQL choses earliest when there is ambiguous time.

For example, in America/Los_Angeles time zone: After Sunday, 06 November, 2011 01:59:59 AM: Clocks were moved backward to become Sunday, 06 November, 2011 01:00:00 AM. When giving 2011-11-06 01:59:59, it can be interpreted to the following two:

  • The time before transition: 2011-11-06 01:59:59 -7 (timestamp: 1320569999)

  • The time after transition: 2011-11-06 01:59:59 -8 (timestamp: 1320573599)

In MySQL:

mysql> SET time_zone = 'America/Los_Angeles';
Query OK, 0 rows affected (0.03 sec)

mysql> select unix_timestamp('2011-11-06 01:59:59');
+---------------------------------------+
| unix_timestamp('2011-11-06 01:59:59') |
+---------------------------------------+
|                            1320569999 |
+---------------------------------------+
1 row in set (0.01 sec)

mysql> select unix_timestamp('2011-11-06 02:00:00');
+---------------------------------------+
| unix_timestamp('2011-11-06 02:00:00') |
+---------------------------------------+
|                            1320573600 |
+---------------------------------------+
1 row in set (0.00 sec)

This comment has been minimized.

Copy link
@huachaohuang

huachaohuang Nov 15, 2018

Contributor

So earliest is equal to single when the conversion is unique?

This comment has been minimized.

Copy link
@breeswish
Copy link
Contributor

left a comment

LGTM

breeswish added 2 commits Nov 16, 2018
@breeswish

This comment has been minimized.

Copy link
Member Author

commented Nov 16, 2018

/run-integration-compatibility-test

@breeswish breeswish merged commit edc6081 into tikv:master Nov 16, 2018
4 checks passed
4 checks passed
DCO All commits are signed off!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-compatibility-test Jenkins job succeeded.
Details
@breeswish breeswish deleted the breeswish:_dst_shift_back branch Nov 16, 2018
@shenli

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

Please cherry-pick this to the release-2.0 and release-2.1.

@breeswish

This comment has been minimized.

Copy link
Member Author

commented Nov 16, 2018

@shenli Got

breeswish added a commit to breeswish/tikv that referenced this pull request Nov 28, 2018
Signed-off-by: Breezewish <breezewish@pingcap.com>
BusyJay added a commit that referenced this pull request Nov 29, 2018
Signed-off-by: Breezewish <breezewish@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.