Skip to content

Commit 4e73cb8

Browse files
holdenksrowen
authored andcommitted
[SPARK-16774][SQL] Fix use of deprecated timestamp constructor & improve timezone handling
## What changes were proposed in this pull request? Removes the deprecated timestamp constructor and incidentally fixes the use which was using system timezone rather than the one specified when working near DST. This change also causes the roundtrip tests to fail since it now actually uses all the timezones near DST boundaries where it didn't before. Note: this is only a partial the solution, longer term we should follow up with https://issues.apache.org/jira/browse/SPARK-16788 to avoid this problem & simplify our timezone handling code. ## How was this patch tested? New tests for two timezones added so even if user timezone happens to coincided with one, the other tests should still fail. Important note: this (temporarily) disables the round trip tests until we can fix the issue more thoroughly. Author: Holden Karau <[email protected]> Closes #14398 from holdenk/SPARK-16774-fix-use-of-deprecated-timestamp-constructor. (cherry picked from commit ab1e761) Signed-off-by: Sean Owen <[email protected]>
1 parent 1523bf6 commit 4e73cb8

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -852,8 +852,10 @@ object DateTimeUtils {
852852

853853
/**
854854
* Lookup the offset for given millis seconds since 1970-01-01 00:00:00 in given timezone.
855+
* TODO: Improve handling of normalization differences.
856+
* TODO: Replace with JSR-310 or similar system - see SPARK-16788
855857
*/
856-
private def getOffsetFromLocalMillis(millisLocal: Long, tz: TimeZone): Long = {
858+
private[sql] def getOffsetFromLocalMillis(millisLocal: Long, tz: TimeZone): Long = {
857859
var guess = tz.getRawOffset
858860
// the actual offset should be calculated based on milliseconds in UTC
859861
val offset = tz.getOffset(millisLocal - guess)
@@ -875,11 +877,11 @@ object DateTimeUtils {
875877
val hh = seconds / 3600
876878
val mm = seconds / 60 % 60
877879
val ss = seconds % 60
878-
val nano = millisOfDay % 1000 * 1000000
879-
880-
// create a Timestamp to get the unix timestamp (in UTC)
881-
val timestamp = new Timestamp(year - 1900, month - 1, day, hh, mm, ss, nano)
882-
guess = (millisLocal - timestamp.getTime).toInt
880+
val ms = millisOfDay % 1000
881+
val calendar = Calendar.getInstance(tz)
882+
calendar.set(year, month - 1, day, hh, mm, ss)
883+
calendar.set(Calendar.MILLISECOND, ms)
884+
guess = (millisLocal - calendar.getTimeInMillis()).toInt
883885
}
884886
}
885887
guess

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,8 @@ class DateTimeUtilsSuite extends SparkFunSuite {
551551
val skipped = skipped_days.getOrElse(tz.getID, Int.MinValue)
552552
(-20000 to 20000).foreach { d =>
553553
if (d != skipped) {
554-
assert(millisToDays(daysToMillis(d)) === d)
554+
assert(millisToDays(daysToMillis(d)) === d,
555+
s"Round trip of ${d} did not work in tz ${tz}")
555556
}
556557
}
557558
}

0 commit comments

Comments
 (0)