Skip to content

[SPARK-31005][SQL] Support time zone ids in casting strings to timestamps #27753

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 4 commits into from
Closed

[SPARK-31005][SQL] Support time zone ids in casting strings to timestamps #27753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40b4e6b
Add tests
MaxGekk Mar 1, 2020
f107331
Support time zone ids in stringToTimestamp
MaxGekk Mar 1, 2020
bdfe5f3
Update the comment for stringToTimestamp
MaxGekk Mar 3, 2020
00a07ab
Add more tests
MaxGekk Mar 4, 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
57 changes: 28 additions & 29 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 @@ -170,28 +170,28 @@ object DateTimeUtils {
* `yyyy-[m]m`
* `yyyy-[m]m-[d]d`
* `yyyy-[m]m-[d]d `
* `yyyy-[m]m-[d]d [h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]`
* `yyyy-[m]m-[d]d [h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]Z`
* `yyyy-[m]m-[d]d [h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]-[h]h:[m]m`
* `yyyy-[m]m-[d]d [h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]+[h]h:[m]m`
* `yyyy-[m]m-[d]dT[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]`
* `yyyy-[m]m-[d]dT[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]Z`
* `yyyy-[m]m-[d]dT[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]-[h]h:[m]m`
* `yyyy-[m]m-[d]dT[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]+[h]h:[m]m`
* `[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]`
* `[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]Z`
* `[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]-[h]h:[m]m`
* `[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]+[h]h:[m]m`
* `T[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]`
* `T[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]Z`
* `T[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]-[h]h:[m]m`
* `T[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us]+[h]h:[m]m`
* `yyyy-[m]m-[d]d [h]h:[m]m:[s]s.[ms][ms][ms][us][us][us][zone_id]`
* `yyyy-[m]m-[d]dT[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us][zone_id]`
* `[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us][zone_id]`
* `T[h]h:[m]m:[s]s.[ms][ms][ms][us][us][us][zone_id]`
*
* where `zone_id` should have one of the forms:
* - Z - Zulu time zone UTC+0
* - +|-[h]h:[m]m
* - A short id, see https://docs.oracle.com/javase/8/docs/api/java/time/ZoneId.html#SHORT_IDS
* - An id with one of the prefixes UTC+, UTC-, GMT+, GMT-, UT+ or UT-,
* and a suffix in the formats:
* - +|-h[h]
* - +|-hh[:]mm
* - +|-hh:mm:ss
* - +|-hhmmss
* - Region-based zone IDs in the form `area/city`, such as `Europe/Paris`
*/
def stringToTimestamp(s: UTF8String, timeZoneId: ZoneId): Option[SQLTimestamp] = {
if (s == null) {
return None
}
var tz: Option[Byte] = None
var tz: Option[String] = None
val segments: Array[Int] = Array[Int](1, 1, 1, 0, 0, 0, 0, 0, 0)
var i = 0
var currentSegmentValue = 0
Expand Down Expand Up @@ -242,22 +242,21 @@ object DateTimeUtils {
return None
}
} else if (i == 5 || i == 6) {
if (b == 'Z') {
if (b == '-' || b == '+') {
Copy link
Member Author

@MaxGekk MaxGekk Mar 2, 2020

Choose a reason for hiding this comment

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

getZoneId() is able to handle zone offsets w/ prefix - and + but it doesn't support the format 7:3 like in

spark/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala

Line 164 in ffc0935

checkCastStringToTimestamp("2015-03-18T12:03:17+7:3", new Timestamp(c.getTimeInMillis))
. So, I have to keep the code for backward compatibility.

segments(i) = currentSegmentValue
currentSegmentValue = 0
i += 1
tz = Some(43)
} else if (b == '-' || b == '+') {
tz = Some(new String(bytes, j, 1))
Copy link
Contributor

@cloud-fan cloud-fan Mar 4, 2020

Choose a reason for hiding this comment

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

why not just b.toChar.toString?

Copy link
Member Author

@MaxGekk MaxGekk Mar 4, 2020

Choose a reason for hiding this comment

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

Just for consistency with another change

} else if (b == '.' && i == 5) {
segments(i) = currentSegmentValue
currentSegmentValue = 0
i += 1
tz = Some(b)
} else if (b == '.' && i == 5) {
} else {
segments(i) = currentSegmentValue
currentSegmentValue = 0
i += 1
} else {
return None
tz = Some(new String(bytes, j, bytes.length - j))
j = bytes.length - 1
}
if (i == 6 && b != '.') {
i += 1
Expand Down Expand Up @@ -297,11 +296,11 @@ object DateTimeUtils {
digitsMilli -= 1
}
try {
val zoneId = if (tz.isEmpty) {
timeZoneId
} else {
val sign = if (tz.get.toChar == '-') -1 else 1
ZoneOffset.ofHoursMinutes(sign * segments(7), sign * segments(8))
val zoneId = tz match {
case None => timeZoneId
case Some("+") => ZoneOffset.ofHoursMinutes(segments(7), segments(8))
case Some("-") => ZoneOffset.ofHoursMinutes(-segments(7), -segments(8))
case Some(zoneName: String) => getZoneId(zoneName.trim)
}
val nanoseconds = MICROSECONDS.toNanos(segments(6))
val localTime = LocalTime.of(segments(3), segments(4), segments(5), nanoseconds.toInt)
Expand Down
23 changes: 20 additions & 3 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 @@ -183,24 +183,29 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers with SQLHelper {
var zoneId = getZoneId("GMT-13:53")
expected = Option(date(2015, 3, 18, 12, 3, 17, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17-13:53", expected)
checkStringToTimestamp("2015-03-18T12:03:17GMT-13:53", expected)

zoneId = getZoneId("UTC")
expected = Option(date(2015, 3, 18, 12, 3, 17, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17Z", expected)
checkStringToTimestamp("2015-03-18 12:03:17Z", expected)
checkStringToTimestamp("2015-03-18 12:03:17UTC", expected)

zoneId = getZoneId("GMT-01:00")
expected = Option(date(2015, 3, 18, 12, 3, 17, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17-1:0", expected)
checkStringToTimestamp("2015-03-18T12:03:17-01:00", expected)
checkStringToTimestamp("2015-03-18T12:03:17GMT-01:00", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(date(2015, 3, 18, 12, 3, 17, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17+07:30", expected)
checkStringToTimestamp("2015-03-18T12:03:17 GMT+07:30", expected)

zoneId = getZoneId("GMT+07:03")
expected = Option(date(2015, 3, 18, 12, 3, 17, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17+07:03", expected)
checkStringToTimestamp("2015-03-18T12:03:17GMT+07:03", expected)

// tests for the string including milliseconds.
expected = Option(date(2015, 3, 18, 12, 3, 17, 123000, zid = zid))
Expand All @@ -213,38 +218,45 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers with SQLHelper {
expected = Option(date(2015, 3, 18, 12, 3, 17, 456000, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.456Z", expected)
checkStringToTimestamp("2015-03-18 12:03:17.456Z", expected)
checkStringToTimestamp("2015-03-18 12:03:17.456 UTC", expected)

zoneId = getZoneId("GMT-01:00")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123000, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.123-1:0", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123-01:00", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123 GMT-01:00", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123000, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.123+07:30", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123 GMT+07:30", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123000, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.123+07:30", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123GMT+07:30", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123121, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.123121+7:30", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123121 GMT+0730", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123120, zid = zoneId))
Copy link
Contributor

@cloud-fan cloud-fan Mar 4, 2020

Choose a reason for hiding this comment

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

why drop this test? 123120 is different from 123121

Copy link
Member Author

@MaxGekk MaxGekk Mar 4, 2020

Choose a reason for hiding this comment

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

reverted it back, and added more tests

checkStringToTimestamp("2015-03-18T12:03:17.12312+7:30", expected)
checkStringToTimestamp("2015-03-18T12:03:17.12312 UT+07:30", expected)

expected = Option(time(18, 12, 15, zid = zid))
checkStringToTimestamp("18:12:15", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(time(18, 12, 15, 123120, zid = zoneId))
checkStringToTimestamp("T18:12:15.12312+7:30", expected)
checkStringToTimestamp("T18:12:15.12312 UTC+07:30", expected)

zoneId = getZoneId("GMT+07:30")
expected = Option(time(18, 12, 15, 123120, zid = zoneId))
checkStringToTimestamp("18:12:15.12312+7:30", expected)
checkStringToTimestamp("18:12:15.12312 GMT+07:30", expected)

expected = Option(date(2011, 5, 6, 7, 8, 9, 100000, zid = zid))
checkStringToTimestamp("2011-05-06 07:08:09.1000", expected)
Expand All @@ -270,8 +282,13 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers with SQLHelper {
// Truncating the fractional seconds
zoneId = getZoneId("GMT+00:00")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123456, zid = zoneId))
checkStringToTimestamp(
"2015-03-18T12:03:17.123456789+0:00", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123456789+0:00", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123456789 UTC+0", expected)
checkStringToTimestamp("2015-03-18T12:03:17.123456789GMT+00:00", expected)

zoneId = getZoneId("Europe/Moscow")
expected = Option(date(2015, 3, 18, 12, 3, 17, 123456, zid = zoneId))
checkStringToTimestamp("2015-03-18T12:03:17.123456 Europe/Moscow", expected)
}
}

Expand Down