Skip to content

[SPARK-30760][SQL] Port millisToDays and daysToMillis on Java 8 time API #27494

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
MaxGekk wants to merge 8 commits into from
Closed

[SPARK-30760][SQL] Port millisToDays and daysToMillis on Java 8 time API #27494

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1211f32
Port daysToMillis()
MaxGekk Feb 7, 2020
9a2b7be
Port millisToDays()
MaxGekk Feb 7, 2020
cc4c07d
Pass ZoneId to millisToDays() and daysToMillis()
MaxGekk Feb 7, 2020
1890589
Use Java 8 time API classes in SQLQueryTestSuite
MaxGekk Feb 8, 2020
6a64946
Fix DateTimeUtilsSuite
MaxGekk Feb 8, 2020
cb37fe3
Replace 2933263 by LocalDate.of(10001, 1, 1).toEpochDay
MaxGekk Feb 10, 2020
e17762f
Merge remote-tracking branch 'remotes/origin/master' into millis-2-da…
MaxGekk Feb 12, 2020
e5d463f
Fix merge conflicts
MaxGekk Feb 12, 2020
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
36 changes: 4 additions & 32 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 @@ -71,10 +71,8 @@ object DateTimeUtils {
}

def millisToDays(millisUtc: Long, timeZone: TimeZone): SQLDate = {
// SPARK-6785: use Math.floorDiv so negative number of days (dates before 1970)
// will correctly work as input for function toJavaDate(Int)
val millisLocal = millisUtc + timeZone.getOffset(millisUtc)
Math.floorDiv(millisLocal, MILLIS_PER_DAY).toInt
val instant = microsToInstant(Math.multiplyExact(millisUtc, MICROS_PER_MILLIS))
localDateToDays(LocalDateTime.ofInstant(instant, timeZone.toZoneId).toLocalDate)
}

// reverse of millisToDays
Expand All @@ -83,8 +81,8 @@ object DateTimeUtils {
}

def daysToMillis(days: SQLDate, timeZone: TimeZone): Long = {
val millisLocal = days.toLong * MILLIS_PER_DAY
millisLocal - getOffsetFromLocalMillis(millisLocal, timeZone)
val instant = daysToLocalDate(days).atStartOfDay(timeZone.toZoneId).toInstant
instantToMicros(instant) / MICROS_PER_MILLIS
}

// Converts Timestamp to string according to Hive TimestampWritable convention.
Expand Down Expand Up @@ -768,32 +766,6 @@ 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[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)
if (offset != guess) {
guess = tz.getOffset(millisLocal - offset)
if (guess != offset) {
// fallback to do the reverse lookup using java.time.LocalDateTime
// this should only happen near the start or end of DST
val localDate = LocalDate.ofEpochDay(MILLISECONDS.toDays(millisLocal))
val localTime = LocalTime.ofNanoOfDay(MILLISECONDS.toNanos(
Math.floorMod(millisLocal, MILLIS_PER_DAY)))
val localDateTime = LocalDateTime.of(localDate, localTime)
val millisEpoch = localDateTime.atZone(tz.toZoneId).toInstant.toEpochMilli

guess = (millisLocal - millisEpoch).toInt
}
}
guess
}

/**
* Convert the timestamp `ts` from one timezone to another.
*
Expand Down
2 changes: 1 addition & 1 deletion ...alyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
// Valid range of DateType is [0001-01-01, 9999-12-31]
val maxMonthInterval = 10000 * 12
checkEvaluation(
AddMonths(Literal(Date.valueOf("0001-01-01")), Literal(maxMonthInterval)), 2933261)
AddMonths(Literal(LocalDate.parse("0001-01-01")), Literal(maxMonthInterval)), 2933263)

@MaxGekk MaxGekk Feb 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had to replace Date.valueOf by LocalDate.parse due to migration on another calendar from Julian to Gregorian. The expected number of days is easy to get:

val a = LocalDate.of(10001, 1, 1).toEpochDay
println(a)
2933263

@cloud-fan cloud-fan Feb 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we place the hardcoded 2933263 with LocalDate.of(10001, 1, 1).toEpochDay?

checkEvaluation(
AddMonths(Literal(Date.valueOf("9999-12-31")), Literal(-1 * maxMonthInterval)), -719529)
// Test evaluation results between Interpreted mode and Codegen mode
Expand Down