srowen commented on a change in pull request #23815: [SPARK-26908][SQL] Fix
DateTimeUtils.toMillis and millisToDays
URL: https://github.com/apache/spark/pull/23815#discussion_r257517562
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
##########
@@ -85,7 +85,7 @@ object DateTimeUtils {
// SPARK-6785: use Math.floor so negative number of days (dates before
1970)
// will correctly work as input for function toJavaDate(Int)
val millisLocal = millisUtc + timeZone.getOffset(millisUtc)
- Math.floor(millisLocal.toDouble / MILLIS_PER_DAY).toInt
+ Math.floorDiv(millisLocal, MILLIS_PER_DAY).toInt
Review comment:
Nevermind, I misunderstood `floorDiv` (I had never seen this method!) I had
it backwards; integer division truncates towards 0 when negative, but
`floorDiv` doesn't.
The original change here had some tests that in theory were asserting the
current behavior. I wonder if they'll fail.
There are also comments in the second change about what the result should be
in millis vs micros. Should that be a test case? It doesn't look like it is,
and it would help verify the fix.
----------------------------------------------------------------
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]
