Skip to content

[SPARK-17914][SQL] Fix parsing of timestamp strings with nanoseconds #18252

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
aokolnychyi wants to merge 3 commits into from
Closed

[SPARK-17914][SQL] Fix parsing of timestamp strings with nanoseconds #18252

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f232a7
[SPARK-17914][SQL] Fix parsing of timestamp strings with nanoseconds
Jun 8, 2017
4d057c9
[SPARK-17914][SQL] Fix parsing of timestamp strings with nanoseconds
Jun 9, 2017
a498f83
[SPARK-17914][SQL] Added a comment about truncation
Jun 10, 2017
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
12 changes: 6 additions & 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 @@ -32,7 +32,7 @@ import org.apache.spark.unsafe.types.UTF8String
* Helper functions for converting between internal and external date and time representations.
* Dates are exposed externally as java.sql.Date and are represented internally as the number of
* dates since the Unix epoch (1970-01-01). Timestamps are exposed externally as java.sql.Timestamp
* and are stored internally as longs, which are capable of storing timestamps with 100 nanosecond
* and are stored internally as longs, which are capable of storing timestamps with microsecond
Copy link
Contributor

@rxin rxin Jun 10, 2017

Choose a reason for hiding this comment

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

100 ns is different from micro, isn't it?

Copy link
Contributor Author

@aokolnychyi aokolnychyi Jun 10, 2017

Choose a reason for hiding this comment

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

Sure, but the previous comment, which was introduced in this commit, was no longer correct. The logic was changed in this commit and now it is up to microseconds.

* precision.
*/
object DateTimeUtils {
Expand Down Expand Up @@ -399,13 +399,13 @@ object DateTimeUtils {
digitsMilli += 1
}

if (!justTime && isInvalidDate(segments(0), segments(1), segments(2))) {
return None
while (digitsMilli > 6) {
Copy link
Contributor

@wzhfy wzhfy Jun 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

add a comment indicating we are truncating the nanosecond part and its lossy?

Copy link
Contributor Author

@aokolnychyi aokolnychyi Jun 10, 2017

Choose a reason for hiding this comment

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

@wzhfy done

segments(6) /= 10
digitsMilli -= 1
}

// Instead of return None, we truncate the fractional seconds to prevent inserting NULL
if (segments(6) > 999999) {
segments(6) = segments(6).toString.take(6).toInt
if (!justTime && isInvalidDate(segments(0), segments(1), segments(2))) {
return None
}

if (segments(3) < 0 || segments(3) > 23 || segments(4) < 0 || segments(4) > 59 ||
Expand Down
16 changes: 16 additions & 0 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 @@ -34,6 +34,22 @@ class DateTimeUtilsSuite extends SparkFunSuite {
((timestamp + tz.getOffset(timestamp)) / MILLIS_PER_DAY).toInt
}

test("nanoseconds truncation") {
def checkStringToTimestamp(originalTime: String, expectedParsedTime: String) {
val parsedTimestampOp = DateTimeUtils.stringToTimestamp(UTF8String.fromString(originalTime))
assert(parsedTimestampOp.isDefined, "timestamp with nanoseconds was not parsed correctly")
assert(DateTimeUtils.timestampToString(parsedTimestampOp.get) === expectedParsedTime)
}

checkStringToTimestamp("2015-01-02 00:00:00.123456789", "2015-01-02 00:00:00.123456")
checkStringToTimestamp("2015-01-02 00:00:00.100000009", "2015-01-02 00:00:00.1")
checkStringToTimestamp("2015-01-02 00:00:00.000050000", "2015-01-02 00:00:00.00005")
checkStringToTimestamp("2015-01-02 00:00:00.12005", "2015-01-02 00:00:00.12005")
checkStringToTimestamp("2015-01-02 00:00:00.100", "2015-01-02 00:00:00.1")
checkStringToTimestamp("2015-01-02 00:00:00.000456789", "2015-01-02 00:00:00.000456")
checkStringToTimestamp("1950-01-02 00:00:00.000456789", "1950-01-02 00:00:00.000456")
}

test("timestamp and us") {
val now = new Timestamp(System.currentTimeMillis())
now.setNanos(1000)
Expand Down