MaxGekk commented on a change in pull request #23878: [SPARK-26978][CORE] Avoid
magic time constants
URL: https://github.com/apache/spark/pull/23878#discussion_r259615031
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
##########
@@ -604,12 +606,12 @@ object DateTimeUtils {
}
// using milliseconds can cause precision loss with more than 8 digits
// we follow Hive's implementation which uses seconds
- val secondsInDay1 = (millis1 - daysToMillis(date1, timeZone)) / 1000L
- val secondsInDay2 = (millis2 - daysToMillis(date2, timeZone)) / 1000L
+ val secondsInDay1 = MILLISECONDS.toSeconds(millis1 - daysToMillis(date1,
timeZone))
+ val secondsInDay2 = MILLISECONDS.toSeconds(millis2 - daysToMillis(date2,
timeZone))
val secondsDiff = (dayInMonth1 - dayInMonth2) * SECONDS_PER_DAY +
secondsInDay1 - secondsInDay2
- // 2678400D is the number of seconds in 31 days
- // every month is considered to be 31 days long in this function
- val diff = monthDiff + secondsDiff / 2678400D
+ val daysInMonth = 31D
+ val secondsInMonth = daysInMonth * DAYS.toSeconds(1)
Review comment:
> Can this be `MONTHS.toSeconds
I haven't found in java.time API how to perform the conversion directly. The
problem is number of days in month is not fixed, so, all methods I have tried
return an error.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
