[GitHub] [spark] cloud-fan commented on a change in pull request #28787: [SPARK-31959][SQL] Fix Gregorian-Julian micros rebasing while switching standard time zone offset
cloud-fan commented on a change in pull request #28787:
URL: https://github.com/apache/spark/pull/28787#discussion_r438577268
##
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala
##
@@ -409,4 +409,31 @@ class RebaseDateTimeSuite extends SparkFunSuite with
Matchers with SQLHelper {
}
}
}
+
+ test("SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945") {
+// The 'Asia/Hong_Kong' time zone switched from 'Japan Standard Time' (JST
= UTC+9)
+// to 'Hong Kong Time' (HKT = UTC+8). After Sunday, 18 November, 1945
01:59:59 AM,
+// 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")
+withDefaultTimeZone(hkZid) {
+ val ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
+ val earlierMicros =
instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+ val laterMicros =
instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+ assert(earlierMicros + MICROS_PER_HOUR === laterMicros)
+ val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkZid,
earlierMicros)
+ val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkZid,
laterMicros)
+ def toTsStr(micros: Long): String = toJavaTimestamp(micros).toString
+ val expected = "1945-11-18 01:30:00.0"
+ assert(toTsStr(rebasedEarlierMicros) === expected)
+ assert(toTsStr(rebasedLaterMicros) === expected)
+ assert(rebasedEarlierMicros + MICROS_PER_HOUR === rebasedLaterMicros)
+ // Check optimized rebasing
Review comment:
so this relies on the JVM system timezone is not `Asia/Hong_Kong`?
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28787: [SPARK-31959][SQL] Fix Gregorian-Julian micros rebasing while switching standard time zone offset
cloud-fan commented on a change in pull request #28787:
URL: https://github.com/apache/spark/pull/28787#discussion_r438550078
##
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/RebaseDateTime.scala
##
@@ -326,20 +326,34 @@ object RebaseDateTime {
*/
private[sql] def rebaseGregorianToJulianMicros(zoneId: ZoneId, micros:
Long): Long = {
val instant = microsToInstant(micros)
-var ldt = instant.atZone(zoneId).toLocalDateTime
+val zonedDateTime = instant.atZone(zoneId)
+var ldt = zonedDateTime.toLocalDateTime
if (ldt.isAfter(julianEndTs) && ldt.isBefore(gregorianStartTs)) {
ldt = LocalDateTime.of(gregorianStartDate, ldt.toLocalTime)
}
val cal = new Calendar.Builder()
- // `gregory` is a hybrid calendar that supports both
- // the Julian and Gregorian calendar systems
+ // `gregory` is a hybrid calendar that supports both the Julian and
Gregorian calendar systems
.setCalendarType("gregory")
.setDate(ldt.getYear, ldt.getMonthValue - 1, ldt.getDayOfMonth)
.setTimeOfDay(ldt.getHour, ldt.getMinute, ldt.getSecond)
- // Local time-line can overlaps, such as at an autumn daylight savings
cutover.
- // This setting selects the original local timestamp mapped to the given
`micros`.
- .set(Calendar.DST_OFFSET,
zoneId.getRules.getDaylightSavings(instant).toMillis.toInt)
.build()
+// A local timestamp can have 2 instants in the cases of switching from:
+// 1. Summer to winter time.
+// 2. One standard time zone to another one. For example, Asia/Hong_Kong
switched from JST
+// to HKT on 18 November, 1945 01:59:59 AM.
+// Below we check that the original `instant` is earlier or later instant.
If it is an earlier
+// instant, we take the standard and DST offsets of the previous day
otherwise of the next one.
+val trans = zoneId.getRules.getTransition(ldt)
+if (trans != null && trans.isOverlap) {
Review comment:
when will we go into this expensive branch?
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28787: [SPARK-31959][SQL] Fix Gregorian-Julian micros rebasing while switching standard time zone offset
cloud-fan commented on a change in pull request #28787:
URL: https://github.com/apache/spark/pull/28787#discussion_r438549793
##
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/RebaseDateTimeSuite.scala
##
@@ -409,4 +409,31 @@ class RebaseDateTimeSuite extends SparkFunSuite with
Matchers with SQLHelper {
}
}
}
+
+ test("SPARK-31959: JST -> HKT at Asia/Hong_Kong in 1945") {
+// The 'Asia/Hong_Kong' time zone switched from 'Japan Standard Time' (JST
= UTC+9)
+// to 'Hong Kong Time' (HKT = UTC+8). After Sunday, 18 November, 1945
01:59:59 AM,
+// 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")
+withDefaultTimeZone(hkZid) {
+ val ldt = LocalDateTime.of(1945, 11, 18, 1, 30, 0)
+ val earlierMicros =
instantToMicros(ldt.atZone(hkZid).withEarlierOffsetAtOverlap().toInstant)
+ val laterMicros =
instantToMicros(ldt.atZone(hkZid).withLaterOffsetAtOverlap().toInstant)
+ assert(earlierMicros + MICROS_PER_HOUR === laterMicros)
+ val rebasedEarlierMicros = rebaseGregorianToJulianMicros(hkZid,
earlierMicros)
+ val rebasedLaterMicros = rebaseGregorianToJulianMicros(hkZid,
laterMicros)
+ def toTsStr(micros: Long): String = toJavaTimestamp(micros).toString
+ val expected = "1945-11-18 01:30:00.0"
+ assert(toTsStr(rebasedEarlierMicros) === expected)
+ assert(toTsStr(rebasedLaterMicros) === expected)
+ assert(rebasedEarlierMicros + MICROS_PER_HOUR === rebasedLaterMicros)
+ // Check optimized rebasing
Review comment:
what do you mean by "optimized rebase"?
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
