You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/13 19:09:40 UTC

[GitHub] [spark] MaxGekk opened a new pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

MaxGekk opened a new pull request #28824:
URL: https://github.com/apache/spark/pull/28824


   ### What changes were proposed in this pull request?
   1. Set the given time zone as the first parameter of `RebaseDateTime`.`rebaseJulianToGregorianMicros()` and `rebaseGregorianToJulianMicros()` to Java 7 `GregorianCalendar`.
   ```scala
       val cal = new Calendar.Builder()
         // `gregory` is a hybrid calendar that supports both the Julian and Gregorian calendar systems
         .setCalendarType("gregory")
       ...
         .setTimeZone(tz)
         .build()
   ```
   This makes the instance of the calendar independent from the default JVM time zone.
   
   2. Change type of the first parameter from `ZoneId` to `TimeZone`. This allows to avoid unnecessary conversion from `TimeZone` to `ZoneId`, for example in
   ```scala
     def rebaseJulianToGregorianMicros(micros: Long): Long = {
       ...
         if (rebaseRecord == null || micros < rebaseRecord.switches(0)) {
           rebaseJulianToGregorianMicros(timeZone.toZoneId, micros)
   ```
   and back to `TimeZone` inside of `rebaseJulianToGregorianMicros(zoneId: ZoneId, ...)`
   
   3. Modify tests in `RebaseDateTimeSuite`, and set the default JVM time zone only for functions that depend on it.
   
   ### Why are the changes needed?
   1. Ignoring passed parameter and use a global variable is bad practice.
   2. Dependency from the global state doesn't allow to run the function in parallel otherwise there is non-zero probability that the function may return wrong result if the default JVM is changed during their execution.
   3. This open opportunity for parallelisation of JSON files generation `gregorian-julian-rebase-micros.json` and `julian-gregorian-rebase-micros.json`. Currently, the tests `generate 'gregorian-julian-rebase-micros.json'` and `generate 'julian-gregorian-rebase-micros.json'` generates the JSON files by iterating over all time zones sequentially w/ step of 1 week. Due to the large step, we can miss some spikes in diffs between 2 calendars (Java 8 Gregorian and Java 7 hybrid calendars) as the PR https://github.com/apache/spark/pull/28787 fixed.
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   By running existing tests from `RebaseDateTimeSuite`.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644566910






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644574854


   **[Test build #124103 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124103/testReport)** for PR 28824 at commit [`381e950`](https://github.com/apache/spark/commit/381e950d6447238a41af6c766f1c7eae81e585a7).
    * This patch **fails due to an unknown error code, -9**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644566345


   **[Test build #124103 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124103/testReport)** for PR 28824 at commit [`381e950`](https://github.com/apache/spark/commit/381e950d6447238a41af6c766f1c7eae81e585a7).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644214555






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643665648


   **[Test build #123982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123982/testReport)** for PR 28824 at commit [`c5306ce`](https://github.com/apache/spark/commit/c5306cedb36c890a07b86ef936a56b6b37ff8fe6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643665648


   **[Test build #123982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123982/testReport)** for PR 28824 at commit [`c5306ce`](https://github.com/apache/spark/commit/c5306cedb36c890a07b86ef936a56b6b37ff8fe6).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on a change in pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28824:
URL: https://github.com/apache/spark/pull/28824#discussion_r440626757



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala
##########
@@ -416,38 +418,39 @@ class RebaseDateTimeSuite extends SparkFunSuite with Matchers with SQLHelper {
     // clocks were moved backward to become Sunday, 18 November, 1945 01:00:00 AM.
     // In this way, the overlap happened w/o Daylight Saving Time.
     val hkZid = getZoneId("Asia/Hong_Kong")
+    var expected = "1945-11-18 01:30:00.0"
+    var ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
+    var earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+    var laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+    var overlapInterval = MICROS_PER_HOUR
+    if (earlierMicros + overlapInterval != laterMicros) {
+      // Old JDK might have an outdated time zone database.
+      // See https://bugs.openjdk.java.net/browse/JDK-8228469: "Hong Kong ... Its 1945 transition
+      // from JST to HKT was on 11-18 at 02:00, not 09-15 at 00:00"
+      expected = "1945-09-14 23:30:00.0"
+      ldt = LocalDateTime.of(1945, 9, 14, 23, 30, 0)
+      earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+      laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+      // If time zone db doesn't have overlapping at all, set the overlap interval to zero.
+      overlapInterval = laterMicros - earlierMicros
+    }
+    val hkTz = TimeZone.getTimeZone(hkZid)
+    val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkTz, earlierMicros)
+    val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkTz, laterMicros)
+    assert(rebasedEarlierMicros + overlapInterval === rebasedLaterMicros)
     withDefaultTimeZone(hkZid) {
-      var expected = "1945-11-18 01:30:00.0"
-      var ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
-      var earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
-      var laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
-      var overlapInterval = MICROS_PER_HOUR
-      if (earlierMicros + overlapInterval != laterMicros) {
-        // Old JDK might have an outdated time zone database.
-        // See https://bugs.openjdk.java.net/browse/JDK-8228469: "Hong Kong ... Its 1945 transition
-        // from JST to HKT was on 11-18 at 02:00, not 09-15 at 00:00"
-        expected = "1945-09-14 23:30:00.0"
-        ldt = LocalDateTime.of(1945, 9, 14, 23, 30, 0)
-        earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
-        laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
-        // If time zone db doesn't have overlapping at all, set the overlap interval to zero.
-        overlapInterval = laterMicros - earlierMicros
-      }
-      val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkZid, earlierMicros)
-      val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkZid, laterMicros)
       def toTsStr(micros: Long): String = toJavaTimestamp(micros).toString
       assert(toTsStr(rebasedEarlierMicros) === expected)
       assert(toTsStr(rebasedLaterMicros) === expected)
-      assert(rebasedEarlierMicros + overlapInterval === rebasedLaterMicros)
       // Check optimized rebasing
       assert(rebaseGregorianToJulianMicros(earlierMicros) === rebasedEarlierMicros)
       assert(rebaseGregorianToJulianMicros(laterMicros) === rebasedLaterMicros)
       // Check reverse rebasing
       assert(rebaseJulianToGregorianMicros(rebasedEarlierMicros) === earlierMicros)
       assert(rebaseJulianToGregorianMicros(rebasedLaterMicros) === laterMicros)
       // Check reverse not-optimized rebasing

Review comment:
       does this rely on JVM timezone?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644574932






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644214555






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644394976


   **[Test build #124061 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124061/testReport)** for PR 28824 at commit [`b42d105`](https://github.com/apache/spark/commit/b42d1056ef2c65728ed51b0fb81e567e58901510).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643665750






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643665750






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644680044


   Merged to master and branch-3.0.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644576078


   **[Test build #124105 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124105/testReport)** for PR 28824 at commit [`9f4f286`](https://github.com/apache/spark/commit/9f4f2863212ec85dcc89de85d65aa9f40217c4be).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644745025


   thanks, merging to master!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644396548






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644731927






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644214042


   **[Test build #124061 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124061/testReport)** for PR 28824 at commit [`b42d105`](https://github.com/apache/spark/commit/b42d1056ef2c65728ed51b0fb81e567e58901510).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644576078


   **[Test build #124105 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124105/testReport)** for PR 28824 at commit [`9f4f286`](https://github.com/apache/spark/commit/9f4f2863212ec85dcc89de85d65aa9f40217c4be).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644730917


   **[Test build #124105 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124105/testReport)** for PR 28824 at commit [`9f4f286`](https://github.com/apache/spark/commit/9f4f2863212ec85dcc89de85d65aa9f40217c4be).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644396548






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643693966






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643693752


   **[Test build #123982 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123982/testReport)** for PR 28824 at commit [`c5306ce`](https://github.com/apache/spark/commit/c5306cedb36c890a07b86ef936a56b6b37ff8fe6).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644566910






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] cloud-fan closed pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #28824:
URL: https://github.com/apache/spark/pull/28824


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644576611






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644574942


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124103/
   Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644214042


   **[Test build #124061 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124061/testReport)** for PR 28824 at commit [`b42d105`](https://github.com/apache/spark/commit/b42d1056ef2c65728ed51b0fb81e567e58901510).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644731927






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644566345


   **[Test build #124103 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124103/testReport)** for PR 28824 at commit [`381e950`](https://github.com/apache/spark/commit/381e950d6447238a41af6c766f1c7eae81e585a7).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644576611






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644574932


   Merged build finished. Test FAILed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643693966






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon removed a comment on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
HyukjinKwon removed a comment on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-644680044


   Merged to master and branch-3.0.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] MaxGekk commented on pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #28824:
URL: https://github.com/apache/spark/pull/28824#issuecomment-643804694


   @cloud-fan @HyukjinKwon Please, review this PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] MaxGekk commented on a change in pull request #28824: [SPARK-31984][SQL] Make micros rebasing functions via local timestamps pure

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #28824:
URL: https://github.com/apache/spark/pull/28824#discussion_r440628505



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala
##########
@@ -416,38 +418,39 @@ class RebaseDateTimeSuite extends SparkFunSuite with Matchers with SQLHelper {
     // clocks were moved backward to become Sunday, 18 November, 1945 01:00:00 AM.
     // In this way, the overlap happened w/o Daylight Saving Time.
     val hkZid = getZoneId("Asia/Hong_Kong")
+    var expected = "1945-11-18 01:30:00.0"
+    var ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
+    var earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+    var laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+    var overlapInterval = MICROS_PER_HOUR
+    if (earlierMicros + overlapInterval != laterMicros) {
+      // Old JDK might have an outdated time zone database.
+      // See https://bugs.openjdk.java.net/browse/JDK-8228469: "Hong Kong ... Its 1945 transition
+      // from JST to HKT was on 11-18 at 02:00, not 09-15 at 00:00"
+      expected = "1945-09-14 23:30:00.0"
+      ldt = LocalDateTime.of(1945, 9, 14, 23, 30, 0)
+      earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+      laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+      // If time zone db doesn't have overlapping at all, set the overlap interval to zero.
+      overlapInterval = laterMicros - earlierMicros
+    }
+    val hkTz = TimeZone.getTimeZone(hkZid)
+    val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkTz, earlierMicros)
+    val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkTz, laterMicros)
+    assert(rebasedEarlierMicros + overlapInterval === rebasedLaterMicros)
     withDefaultTimeZone(hkZid) {
-      var expected = "1945-11-18 01:30:00.0"
-      var ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
-      var earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
-      var laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
-      var overlapInterval = MICROS_PER_HOUR
-      if (earlierMicros + overlapInterval != laterMicros) {
-        // Old JDK might have an outdated time zone database.
-        // See https://bugs.openjdk.java.net/browse/JDK-8228469: "Hong Kong ... Its 1945 transition
-        // from JST to HKT was on 11-18 at 02:00, not 09-15 at 00:00"
-        expected = "1945-09-14 23:30:00.0"
-        ldt = LocalDateTime.of(1945, 9, 14, 23, 30, 0)
-        earlierMicros = instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
-        laterMicros = instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
-        // If time zone db doesn't have overlapping at all, set the overlap interval to zero.
-        overlapInterval = laterMicros - earlierMicros
-      }
-      val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkZid, earlierMicros)
-      val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkZid, laterMicros)
       def toTsStr(micros: Long): String = toJavaTimestamp(micros).toString
       assert(toTsStr(rebasedEarlierMicros) === expected)
       assert(toTsStr(rebasedLaterMicros) === expected)
-      assert(rebasedEarlierMicros + overlapInterval === rebasedLaterMicros)
       // Check optimized rebasing
       assert(rebaseGregorianToJulianMicros(earlierMicros) === rebasedEarlierMicros)
       assert(rebaseGregorianToJulianMicros(laterMicros) === rebasedLaterMicros)
       // Check reverse rebasing
       assert(rebaseJulianToGregorianMicros(rebasedEarlierMicros) === earlierMicros)
       assert(rebaseJulianToGregorianMicros(rebasedLaterMicros) === laterMicros)
       // Check reverse not-optimized rebasing

Review comment:
       No, they don't. Let me move them out of the default JVM time zone. Thanks.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org