spark-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sro...@apache.org
Subject spark git commit: [SPARK-16774][SQL] Fix use of deprecated timestamp constructor & improve timezone handling
Date Mon, 01 Aug 2016 20:57:21 GMT
Repository: spark
Updated Branches:
  refs/heads/branch-2.0 1523bf69a -> 4e73cb8eb


[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 <holden@us.ibm.com>

Closes #14398 from holdenk/SPARK-16774-fix-use-of-deprecated-timestamp-constructor.

(cherry picked from commit ab1e761f9691b41385e2ed2202c5a671c63c963d)
Signed-off-by: Sean Owen <sowen@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/4e73cb8e
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/4e73cb8e
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/4e73cb8e

Branch: refs/heads/branch-2.0
Commit: 4e73cb8ebdb0dcb1be4dce562bac9214e9905b8e
Parents: 1523bf6
Author: Holden Karau <holden@us.ibm.com>
Authored: Mon Aug 1 13:57:05 2016 -0700
Committer: Sean Owen <sowen@cloudera.com>
Committed: Mon Aug 1 13:57:19 2016 -0700

----------------------------------------------------------------------
 .../spark/sql/catalyst/util/DateTimeUtils.scala       | 14 ++++++++------
 .../spark/sql/catalyst/util/DateTimeUtilsSuite.scala  |  3 ++-
 2 files changed, 10 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/4e73cb8e/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
index df480a1..0b643a5 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
@@ -852,8 +852,10 @@ object DateTimeUtils {
 
   /**
    * Lookup the offset for given millis seconds since 1970-01-01 00:00:00 in given timezone.
+   * TODO: Improve handling of normalization differences.
+   * TODO: Replace with JSR-310 or similar system - see SPARK-16788
    */
-  private def getOffsetFromLocalMillis(millisLocal: Long, tz: TimeZone): Long = {
+  private[sql] def getOffsetFromLocalMillis(millisLocal: Long, tz: TimeZone): Long = {
     var guess = tz.getRawOffset
     // the actual offset should be calculated based on milliseconds in UTC
     val offset = tz.getOffset(millisLocal - guess)
@@ -875,11 +877,11 @@ object DateTimeUtils {
         val hh = seconds / 3600
         val mm = seconds / 60 % 60
         val ss = seconds % 60
-        val nano = millisOfDay % 1000 * 1000000
-
-        // create a Timestamp to get the unix timestamp (in UTC)
-        val timestamp = new Timestamp(year - 1900, month - 1, day, hh, mm, ss, nano)
-        guess = (millisLocal - timestamp.getTime).toInt
+        val ms = millisOfDay % 1000
+        val calendar = Calendar.getInstance(tz)
+        calendar.set(year, month - 1, day, hh, mm, ss)
+        calendar.set(Calendar.MILLISECOND, ms)
+        guess = (millisLocal - calendar.getTimeInMillis()).toInt
       }
     }
     guess

http://git-wip-us.apache.org/repos/asf/spark/blob/4e73cb8e/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
index 059a5b7..4f516d0 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
@@ -551,7 +551,8 @@ class DateTimeUtilsSuite extends SparkFunSuite {
         val skipped = skipped_days.getOrElse(tz.getID, Int.MinValue)
         (-20000 to 20000).foreach { d =>
           if (d != skipped) {
-            assert(millisToDays(daysToMillis(d)) === d)
+            assert(millisToDays(daysToMillis(d)) === d,
+              s"Round trip of ${d} did not work in tz ${tz}")
           }
         }
       }


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


Mime
View raw message