[SPARK-31959][SQL][3.0] Fix Gregorian-Julian micros rebasing while switching standard time zone offset #28809

MaxGekk · 2020-06-12T07:55:05Z

What changes were proposed in this pull request?

Fix the bug in microseconds rebasing during transitions from one standard time zone offset to another one. In the PR, I propose to change the implementation of rebaseGregorianToJulianMicros which performs rebasing via local timestamps. In the case of overlapping:

Check that the original instant belongs to earlier or later instant of overlapped local timestamp.
If it is an earlier instant, take zone and DST offsets from the previous day otherwise
Set time zone offsets to Julian timestamp from the next day.

Note: The fix assumes that transitions cannot happen more often than once per 2 days.

Adopt the test "SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945" to outdated tzdb. Old JDK can have outdated time zone database in which Asia/Hong_Kong doesn't have timestamp overlapping in 1945 at all.

Why are the changes needed?

Current implementation handles timestamps overlapping only during daylight saving time but overlapping can happen also during transition from one standard time zone to another one. For example in the case of Asia/Hong_Kong, the time zone switched from Japan Standard Time (UTC+9) to Hong Kong Time (UTC+8) on Sunday, 18 November, 1945 01:59:59 AM. The changes allow to handle the special case as well.
To fix the test failures on old JDK w/ outdated tzdb like on Jenkins machine research-jenkins-worker-09.

Does this PR introduce any user-facing change?

It might affect micros rebasing in before common era when not-optimised version of rebaseGregorianToJulianMicros() is used directly.

How was this patch tested?

By existing tests in DateTimeUtilsSuite, RebaseDateTimeSuite, DateFunctionsSuite, DateExpressionsSuite and TimestampFormatterSuite.
Added new test to RebaseDateTimeSuite
Regenerated gregorian-julian-rebase-micros.json with the step of 30 minutes, and got the same JSON file. The JSON file isn't affected because previously it was generated with the step of 1 week. And the spike in diffs/switch points during 1 hour of timestamp overlapping wasn't detected.

Authored-by: Max Gekk [email protected]
Signed-off-by: Wenchen Fan [email protected]
(cherry picked from commit c259844)
Signed-off-by: Dongjoon Hyun [email protected]
(cherry picked from commit eae1747)
Signed-off-by: Max Gekk [email protected]

…while switching standard time zone offset Fix the bug in microseconds rebasing during transitions from one standard time zone offset to another one. In the PR, I propose to change the implementation of `rebaseGregorianToJulianMicros` which performs rebasing via local timestamps. In the case of overlapping: 1. Check that the original instant belongs to earlier or later instant of overlapped local timestamp. 2. If it is an earlier instant, take zone and DST offsets from the previous day otherwise 3. Set time zone offsets to Julian timestamp from the next day. Note: The fix assumes that transitions cannot happen more often than once per 2 days. Current implementation handles timestamps overlapping only during daylight saving time but overlapping can happen also during transition from one standard time zone to another one. For example in the case of `Asia/Hong_Kong`, the time zone switched from `Japan Standard Time` (UTC+9) to `Hong Kong Time` (UTC+8) on _Sunday, 18 November, 1945 01:59:59 AM_. The changes allow to handle the special case as well. It might affect micros rebasing in before common era when not-optimised version of `rebaseGregorianToJulianMicros()` is used directly. 1. By existing tests in `DateTimeUtilsSuite`, `RebaseDateTimeSuite`, `DateFunctionsSuite`, `DateExpressionsSuite` and `TimestampFormatterSuite`. 2. Added new test to `RebaseDateTimeSuite` 3. Regenerated `gregorian-julian-rebase-micros.json` with the step of 30 minutes, and got the same JSON file. The JSON file isn't affected because previously it was generated with the step of 1 week. And the spike in diffs/switch points during 1 hour of timestamp overlapping wasn't detected. Closes apache#28787 from MaxGekk/HongKong-tz-1945. Authored-by: Max Gekk <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit c259844) Signed-off-by: Max Gekk <[email protected]>

MaxGekk · 2020-06-12T07:55:54Z

@cloud-fan Please, take a look at the backport of #28787 to branch-3.0

cloud-fan · 2020-06-12T09:28:35Z

do you know what caused the conflicts?

MaxGekk · 2020-06-12T09:34:57Z

do you know what caused the conflicts?

As usual - fromMillis()

SparkQA · 2020-06-12T11:51:27Z

Test build #123906 has finished for PR 28809 at commit 35245c0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2020-06-12T12:49:02Z

Jenkins, retest this, please

gatorsmile · 2020-06-12T18:30:27Z

We need to split such a long Jenkin job to multiple shorter Jenkins Jobs

SparkQA · 2020-06-12T18:41:47Z

Test build #123925 has finished for PR 28809 at commit 35245c0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala

dongjoon-hyun

After some investigations, I found that the failures seem to reported consistently on research-jenkins-worker-09. It might be Amplap Jenkins host issue (Java version or environment). We had better fix the host or make this PR more robust on those problems before merging this PR to branch-3.0.

MaxGekk · 2020-06-15T05:59:24Z

It might be Amplap Jenkins host issue (Java version or environment).

It uses JDK w/ outdated time zone database (not clear from log which version):

JAVA_HOME=/usr/lib/jvm/java-8-oracle

other jenkins machines have:

JAVA_HOME=/usr/java/jdk1.8.0_191

If we are not able to upgrade JDK 1.8 to the recent version, can we have at least the same JDK on all jenkins machines?

MaxGekk · 2020-06-15T06:06:42Z

I am going to skip the test checks if JDK tzdb is outdated and Asia/Hong_Kong doesn't have timestamps overlapping in 1945 at all.

MaxGekk · 2020-06-15T08:48:01Z

I will cherry-pick #28832 here when it will be merged to master.

dongjoon-hyun · 2020-06-15T15:10:13Z

It's merged now. Thanks!

…-> HKT at Asia/Hong_Kong in 1945" to outdated tzdb Old JDK can have outdated time zone database in which `Asia/Hong_Kong` doesn't have timestamp overlapping in 1946 at all. This PR changes the test "SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945" in `RebaseDateTimeSuite`, and makes it tolerant to the case. To fix the test failures on old JDK w/ outdated tzdb like on Jenkins machine `research-jenkins-worker-09`. No By running the test on old JDK Closes apache#28832 from MaxGekk/HongKong-tz-1945-followup. Authored-by: Max Gekk <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit eae1747) Signed-off-by: Max Gekk <[email protected]>

SparkQA · 2020-06-15T20:33:55Z

Test build #124060 has finished for PR 28809 at commit 63898cf.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-06-16T04:56:27Z

thanks, merging to 3.0!

…itching standard time zone offset ### What changes were proposed in this pull request? Fix the bug in microseconds rebasing during transitions from one standard time zone offset to another one. In the PR, I propose to change the implementation of `rebaseGregorianToJulianMicros` which performs rebasing via local timestamps. In the case of overlapping: 1. Check that the original instant belongs to earlier or later instant of overlapped local timestamp. 2. If it is an earlier instant, take zone and DST offsets from the previous day otherwise 3. Set time zone offsets to Julian timestamp from the next day. Note: The fix assumes that transitions cannot happen more often than once per 2 days. Adopt the test "SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945" to outdated tzdb. Old JDK can have outdated time zone database in which Asia/Hong_Kong doesn't have timestamp overlapping in 1945 at all. ### Why are the changes needed? 1. Current implementation handles timestamps overlapping only during daylight saving time but overlapping can happen also during transition from one standard time zone to another one. For example in the case of `Asia/Hong_Kong`, the time zone switched from `Japan Standard Time` (UTC+9) to `Hong Kong Time` (UTC+8) on _Sunday, 18 November, 1945 01:59:59 AM_. The changes allow to handle the special case as well. 2. To fix the test failures on old JDK w/ outdated tzdb like on Jenkins machine `research-jenkins-worker-09`. ### Does this PR introduce _any_ user-facing change? It might affect micros rebasing in before common era when not-optimised version of `rebaseGregorianToJulianMicros()` is used directly. ### How was this patch tested? 1. By existing tests in `DateTimeUtilsSuite`, `RebaseDateTimeSuite`, `DateFunctionsSuite`, `DateExpressionsSuite` and `TimestampFormatterSuite`. 2. Added new test to `RebaseDateTimeSuite` 3. Regenerated `gregorian-julian-rebase-micros.json` with the step of 30 minutes, and got the same JSON file. The JSON file isn't affected because previously it was generated with the step of 1 week. And the spike in diffs/switch points during 1 hour of timestamp overlapping wasn't detected. Authored-by: Max Gekk <max.gekkgmail.com> Signed-off-by: Wenchen Fan <wenchendatabricks.com> (cherry picked from commit c259844) Signed-off-by: Dongjoon Hyun <dongjoonapache.org> (cherry picked from commit eae1747) Signed-off-by: Max Gekk <max.gekkgmail.com> Closes #28809 from MaxGekk/HongKong-tz-1945-3.0. Authored-by: Max Gekk <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…itching standard time zone offset ### What changes were proposed in this pull request? Fix the bug in microseconds rebasing during transitions from one standard time zone offset to another one. In the PR, I propose to change the implementation of `rebaseGregorianToJulianMicros` which performs rebasing via local timestamps. In the case of overlapping: 1. Check that the original instant belongs to earlier or later instant of overlapped local timestamp. 2. If it is an earlier instant, take zone and DST offsets from the previous day otherwise 3. Set time zone offsets to Julian timestamp from the next day. Note: The fix assumes that transitions cannot happen more often than once per 2 days. Adopt the test "SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945" to outdated tzdb. Old JDK can have outdated time zone database in which Asia/Hong_Kong doesn't have timestamp overlapping in 1945 at all. ### Why are the changes needed? 1. Current implementation handles timestamps overlapping only during daylight saving time but overlapping can happen also during transition from one standard time zone to another one. For example in the case of `Asia/Hong_Kong`, the time zone switched from `Japan Standard Time` (UTC+9) to `Hong Kong Time` (UTC+8) on _Sunday, 18 November, 1945 01:59:59 AM_. The changes allow to handle the special case as well. 2. To fix the test failures on old JDK w/ outdated tzdb like on Jenkins machine `research-jenkins-worker-09`. ### Does this PR introduce _any_ user-facing change? It might affect micros rebasing in before common era when not-optimised version of `rebaseGregorianToJulianMicros()` is used directly. ### How was this patch tested? 1. By existing tests in `DateTimeUtilsSuite`, `RebaseDateTimeSuite`, `DateFunctionsSuite`, `DateExpressionsSuite` and `TimestampFormatterSuite`. 2. Added new test to `RebaseDateTimeSuite` 3. Regenerated `gregorian-julian-rebase-micros.json` with the step of 30 minutes, and got the same JSON file. The JSON file isn't affected because previously it was generated with the step of 1 week. And the spike in diffs/switch points during 1 hour of timestamp overlapping wasn't detected. Authored-by: Max Gekk <max.gekkgmail.com> Signed-off-by: Wenchen Fan <wenchendatabricks.com> (cherry picked from commit c259844) Signed-off-by: Dongjoon Hyun <dongjoonapache.org> (cherry picked from commit eae1747) Signed-off-by: Max Gekk <max.gekkgmail.com> Closes apache#28809 from MaxGekk/HongKong-tz-1945-3.0. Authored-by: Max Gekk <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

probot-autolabeler bot added the SQL label Jun 12, 2020

cloud-fan approved these changes Jun 12, 2020

View reviewed changes

dongjoon-hyun reviewed Jun 14, 2020

View reviewed changes

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala Show resolved Hide resolved

dongjoon-hyun requested changes Jun 14, 2020

View reviewed changes

dongjoon-hyun mentioned this pull request Jun 14, 2020

[SPARK-31959][SQL][test-java11] Fix Gregorian-Julian micros rebasing while switching standard time zone offset #28787

Closed

cloud-fan mentioned this pull request Jun 15, 2020

[SPARK-31986][SQL] Fix Julian-Gregorian micros rebasing of overlapping local timestamps #28816

Closed

cloud-fan approved these changes Jun 15, 2020

View reviewed changes

cloud-fan closed this Jun 16, 2020

MaxGekk deleted the HongKong-tz-1945-3.0 branch December 11, 2020 20:26

[SPARK-31959][SQL][3.0] Fix Gregorian-Julian micros rebasing while switching standard time zone offset #28809

[SPARK-31959][SQL][3.0] Fix Gregorian-Julian micros rebasing while switching standard time zone offset #28809

Uh oh!

Conversation

MaxGekk commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

MaxGekk commented Jun 12, 2020

Uh oh!

cloud-fan commented Jun 12, 2020

Uh oh!

MaxGekk commented Jun 12, 2020

Uh oh!

SparkQA commented Jun 12, 2020

Uh oh!

MaxGekk commented Jun 12, 2020

Uh oh!

gatorsmile commented Jun 12, 2020

Uh oh!

SparkQA commented Jun 12, 2020

Uh oh!

Uh oh!

dongjoon-hyun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxGekk commented Jun 15, 2020

Uh oh!

MaxGekk commented Jun 15, 2020

Uh oh!

MaxGekk commented Jun 15, 2020

Uh oh!

dongjoon-hyun commented Jun 15, 2020

Uh oh!

SparkQA commented Jun 15, 2020

Uh oh!

cloud-fan commented Jun 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MaxGekk commented Jun 12, 2020 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading