This is an automated email from the ASF dual-hosted git repository.
srowen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 75c48ac [SPARK-26908][SQL] Fix DateTimeUtils.toMillis and millisToDays
75c48ac is described below
commit 75c48ac36d3c75f405b7ad8fd989ff4ee00eccbb
Author: Maxim Gekk <max.g...@gmail.com>
AuthorDate: Sat Feb 23 11:35:11 2019 -0600
[SPARK-26908][SQL] Fix DateTimeUtils.toMillis and millisToDays
## What changes were proposed in this pull request?
The `DateTimeUtils.toMillis` can produce inaccurate result for some
negative values (timestamps before epoch). The error can be around 1ms. In the
PR, I propose to use `Math.floorDiv` in casting microseconds to milliseconds,
and milliseconds to days since epoch.
## How was this patch tested?
Added new test to `DateTimeUtilsSuite`, and tested by `CastSuite` as well.
Closes #23815 from MaxGekk/micros-to-millis.
Lead-authored-by: Maxim Gekk <max.g...@gmail.com>
Co-authored-by: Maxim Gekk <maxim.g...@databricks.com>
Signed-off-by: Sean Owen <sean.o...@databricks.com>
---
.../scala/org/apache/spark/sql/catalyst/expressions/Cast.scala | 6 ++++--
.../org/apache/spark/sql/catalyst/util/DateTimeUtils.scala | 10 +++++-----
.../apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala | 6 ++++++
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
index a6926d8..b20249f 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
@@ -396,7 +396,9 @@ case class Cast(child: Expression, dataType: DataType,
timeZoneId: Option[String
// converting seconds to us
private[this] def longToTimestamp(t: Long): Long = t * 1000000L
// converting us to seconds
- private[this] def timestampToLong(ts: Long): Long = math.floor(ts.toDouble /
1000000L).toLong
+ private[this] def timestampToLong(ts: Long): Long = {
+ Math.floorDiv(ts, DateTimeUtils.MICROS_PER_SECOND)
+ }
// converting us to seconds in double
private[this] def timestampToDouble(ts: Long): Double = {
ts / 1000000.0
@@ -1072,7 +1074,7 @@ case class Cast(child: Expression, dataType: DataType,
timeZoneId: Option[String
}
private[this] def longToTimeStampCode(l: ExprValue): Block = code"$l *
1000000L"
private[this] def timestampToIntegerCode(ts: ExprValue): Block =
- code"java.lang.Math.floor((double) $ts / 1000000L)"
+ code"java.lang.Math.floorDiv($ts, 1000000L)"
private[this] def timestampToDoubleCode(ts: ExprValue): Block =
code"$ts / 1000000.0"
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 5a432ba..d714d29 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
@@ -77,10 +77,10 @@ object DateTimeUtils {
}
def millisToDays(millisUtc: Long, timeZone: TimeZone): SQLDate = {
- // SPARK-6785: use Math.floor so negative number of days (dates before
1970)
+ // 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.floor(millisLocal.toDouble / MILLIS_PER_DAY).toInt
+ Math.floorDiv(millisLocal, MILLIS_PER_DAY).toInt
}
// reverse of millisToDays
@@ -179,14 +179,14 @@ object DateTimeUtils {
// When the timestamp is negative i.e before 1970, we need to adjust the
millseconds portion.
// Example - 1965-01-01 10:11:12.123456 is represented as
(-157700927876544) in micro precision.
// In millis precision the above needs to be represented as
(-157700927877).
- Math.floor(us.toDouble / MILLIS_PER_SECOND).toLong
+ Math.floorDiv(us, MICROS_PER_MILLIS)
}
/*
- * Converts millseconds since epoch to SQLTimestamp.
+ * Converts milliseconds since epoch to SQLTimestamp.
*/
def fromMillis(millis: Long): SQLTimestamp = {
- millis * 1000L
+ millis * MICROS_PER_MILLIS
}
/**
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 b71790e..e270b91 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
@@ -515,6 +515,7 @@ class DateTimeUtilsSuite extends SparkFunSuite {
val input = TimeUnit.MICROSECONDS.toMillis(date(2015, 12, 31, 16, tz =
TimeZonePST))
assert(millisToDays(input, TimeZonePST) === 16800)
assert(millisToDays(input, TimeZoneGMT) === 16801)
+ assert(millisToDays(-1 * MILLIS_PER_DAY + 1, TimeZoneGMT) == -1)
var expected = TimeUnit.MICROSECONDS.toMillis(date(2015, 12, 31, tz =
TimeZonePST))
assert(daysToMillis(16800, TimeZonePST) === expected)
@@ -541,4 +542,9 @@ class DateTimeUtilsSuite extends SparkFunSuite {
}
}
}
+
+ test("toMillis") {
+ assert(DateTimeUtils.toMillis(-9223372036844776001L) ===
-9223372036844777L)
+ assert(DateTimeUtils.toMillis(-157700927876544L) === -157700927877L)
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org
