Skip to content

[SPARK-16774][SQL] Fix use of deprecated timestamp constructor & improve timezone handling #14398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
holdenk wants to merge 12 commits into from
Closed

[SPARK-16774][SQL] Fix use of deprecated timestamp constructor & improve timezone handling #14398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7c0fd8d
Stop using deprecated Timestamp constructor
holdenk Jul 28, 2016
634a05e
Wait the old timezone code was badnews bears - did stuff based implic…
holdenk Jul 29, 2016
6233b4c
Simplify & disable the roundtrip test - it wasn't really running anyw…
holdenk Jul 29, 2016
e4508e3
not used anymore
holdenk Jul 29, 2016
0385d8b
Just remove the roundtrip test for now - can be added back from git h…
holdenk Jul 29, 2016
5e80a8d
Use standard calendar
holdenk Jul 29, 2016
c01db02
Don't need to capture return value of set
holdenk Jul 29, 2016
ec75f73
Add another TODO with JIRA link
holdenk Jul 29, 2016
7c28252
Merge branch 'master' into SPARK-16774-fix-use-of-deprecated-timestam…
holdenk Jul 29, 2016
8670bb6
Add back tests and add some more notes
holdenk Jul 30, 2016
1109275
nvm about those localmilis based tests - should be covered through to…
holdenk Jul 30, 2016
671f7be
Remove superflous getTime call
holdenk Jul 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
nvm about those localmilis based tests - should be covered through to… 
… UTC tests
  • Loading branch information
@holdenk
holdenk committed Jul 30, 2016
commit 110927562ac57e8c0490c373569a59eedb09c593
7 changes: 1 addition & 6 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,12 +882,7 @@ object DateTimeUtils {
calendar.set(year, month - 1, day, hh, mm, ss)
calendar.set(Calendar.MILLISECOND, ms)
val date = calendar.getTime()
Copy link
Member

@ckadner ckadner Jul 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems unnecessary ... if the intend is to make sure Calendar.computeTime() will be invoked then that will happen when calling calendar.getTimeInMillis() on the next line as well

// TODO decide between return the offset from the calendar:
guess = calendar.get(Calendar.ZONE_OFFSET) + calendar.get(Calendar.DST_OFFSET)
// And returning the difference between timestamps (end up broken in different ways)
// This way appears to return a non-DST value even when in DST - but our round trips tests
// will work.
// guess = (millisLocal - calendar.getTimeInMillis()).toInt
guess = (millisLocal - calendar.getTimeInMillis()).toInt
}
}
guess
Expand Down
32 changes: 0 additions & 32 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,36 +558,4 @@ class DateTimeUtilsSuite extends SparkFunSuite {
}
}
}

test("convert TZ with boundary time pacific") {
val tz = TimeZone.getTimeZone("America/Los_Angeles")
val boundaryDates = List(
1425680000000L, // March 6, 2015 @ 10:13:20 pm
1425779000000L, // March 8, 2015 @ 1:43:20 am
1425780000000L, // March 8, 2015 @ 3:00:00 am
1425781000000L, // March 8, 2015 @ 3:16:40 am
1425783600000L, // March 8, 2015 @ 4:00:00 am
1425880000000L, // March 9, 2015 @ 6:46:40 am
1004580000000L, // November 1, 2001 @ 1:00:00 am
1004580060000L // November 1, 2001 @ 1:01:00 am
)
val offsets = boundaryDates.map(boundaryDate =>
DateTimeUtils.getOffsetFromLocalMillis(boundaryDate, tz))
assert(
List(-28800000, -28800000, -25200000, -25200000, -25200000, -25200000, -28800000, -28800000)
=== offsets)
}

test("convert TZ with boundary time London") {
val tz = TimeZone.getTimeZone("Europe/London")
val boundaryDates = List(
1459040340000L, // March 27, 2016 @ 12:59:00 am
1459040400000L, // March 27, 2016 @ 2:00:00 am
1459040410000L, // March 27, 2016 @ 2:16:40 am
1459126800000L // March 28, 2016 @ 2:00:00 am
)
val offsets = boundaryDates.map(boundaryDate =>
DateTimeUtils.getOffsetFromLocalMillis(boundaryDate, tz))
assert(List(0, 3600000, 3600000, 3600000) == offsets)
}
}