You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2024/08/01 14:29:24 UTC
(spark) branch master updated: [SPARK-49065][SQL] Rebasing in legacy formatters/parsers must support non JVM default time zones
This is an automated email from the ASF dual-hosted git repository.
wenchen 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 63c08632f2d6 [SPARK-49065][SQL] Rebasing in legacy formatters/parsers must support non JVM default time zones
63c08632f2d6 is described below
commit 63c08632f2d6e559bc6a8396e646389e5402c757
Author: Sumeet Varma <su...@databricks.com>
AuthorDate: Thu Aug 1 22:29:08 2024 +0800
[SPARK-49065][SQL] Rebasing in legacy formatters/parsers must support non JVM default time zones
### What changes were proposed in this pull request?
Explicitly pass the overridden timezone parameter to `rebaseJulianToGregorianMicros` and `rebaseGregorianToJulianMicros`.
### Why are the changes needed?
Currently, rebasing timestamp defaults to JVM timezone and so it produces incorrect results when the explicitly over-riden timezone in the TimestampFormatter library is not the same as JVM timezone.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
New UT to capture this scenario
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #47541 from sumeet-db/rebase_time_zone.
Authored-by: Sumeet Varma <su...@databricks.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../sql/catalyst/util/SparkDateTimeUtils.scala | 6 ++++
.../sql/catalyst/util/TimestampFormatter.scala | 12 ++++----
.../catalyst/util/TimestampFormatterSuite.scala | 34 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala
index 0447d813e26a..a6592ad51c65 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala
@@ -251,6 +251,9 @@ trait SparkDateTimeUtils {
def toJavaTimestamp(micros: Long): Timestamp =
toJavaTimestampNoRebase(rebaseGregorianToJulianMicros(micros))
+ def toJavaTimestamp(timeZoneId: String, micros: Long): Timestamp =
+ toJavaTimestampNoRebase(rebaseGregorianToJulianMicros(timeZoneId, micros))
+
/**
* Converts microseconds since the epoch to an instance of `java.sql.Timestamp`.
*
@@ -287,6 +290,9 @@ trait SparkDateTimeUtils {
def fromJavaTimestamp(t: Timestamp): Long =
rebaseJulianToGregorianMicros(fromJavaTimestampNoRebase(t))
+ def fromJavaTimestamp(timeZoneId: String, t: Timestamp): Long =
+ rebaseJulianToGregorianMicros(timeZoneId, fromJavaTimestampNoRebase(t))
+
/**
* Converts an instance of `java.sql.Timestamp` to the number of microseconds since
* 1970-01-01T00:00:00.000000Z.
diff --git a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/TimestampFormatter.scala b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/TimestampFormatter.scala
index 9f57f8375c54..79d627b493fd 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/TimestampFormatter.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/TimestampFormatter.scala
@@ -435,11 +435,11 @@ class LegacyFastTimestampFormatter(
val micros = cal.getMicros()
cal.set(Calendar.MILLISECOND, 0)
val julianMicros = Math.addExact(millisToMicros(cal.getTimeInMillis), micros)
- rebaseJulianToGregorianMicros(julianMicros)
+ rebaseJulianToGregorianMicros(TimeZone.getTimeZone(zoneId), julianMicros)
}
override def format(timestamp: Long): String = {
- val julianMicros = rebaseGregorianToJulianMicros(timestamp)
+ val julianMicros = rebaseGregorianToJulianMicros(TimeZone.getTimeZone(zoneId), timestamp)
cal.setTimeInMillis(Math.floorDiv(julianMicros, MICROS_PER_SECOND) * MILLIS_PER_SECOND)
cal.setMicros(Math.floorMod(julianMicros, MICROS_PER_SECOND))
fastDateFormat.format(cal)
@@ -449,7 +449,7 @@ class LegacyFastTimestampFormatter(
if (ts.getNanos == 0) {
fastDateFormat.format(ts)
} else {
- format(fromJavaTimestamp(ts))
+ format(fromJavaTimestamp(zoneId.getId, ts))
}
}
@@ -473,7 +473,7 @@ class LegacySimpleTimestampFormatter(
}
override def parse(s: String): Long = {
- fromJavaTimestamp(new Timestamp(sdf.parse(s).getTime))
+ fromJavaTimestamp(zoneId.getId, new Timestamp(sdf.parse(s).getTime))
}
override def parseOptional(s: String): Option[Long] = {
@@ -481,12 +481,12 @@ class LegacySimpleTimestampFormatter(
if (date == null) {
None
} else {
- Some(fromJavaTimestamp(new Timestamp(date.getTime)))
+ Some(fromJavaTimestamp(zoneId.getId, new Timestamp(date.getTime)))
}
}
override def format(us: Long): String = {
- sdf.format(toJavaTimestamp(us))
+ sdf.format(toJavaTimestamp(zoneId.getId, us))
}
override def format(ts: Timestamp): String = {
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/TimestampFormatterSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/TimestampFormatterSuite.scala
index 7eac6e0feef6..6223f9aadb59 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/TimestampFormatterSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/TimestampFormatterSuite.scala
@@ -299,6 +299,40 @@ class TimestampFormatterSuite extends DatetimeFormatterSuite {
}
}
+ test("SPARK-49065: rebasing in legacy formatters/parsers with non-default time zone") {
+ val defaultTimeZone = LA
+ withSQLConf(SQLConf.LEGACY_TIME_PARSER_POLICY.key -> LegacyBehaviorPolicy.LEGACY.toString) {
+ outstandingZoneIds.foreach { zoneId =>
+ withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> defaultTimeZone.getId) {
+ withDefaultTimeZone(defaultTimeZone) {
+ withClue(s"zoneId = ${zoneId.getId}") {
+ val formatters = LegacyDateFormats.values.toSeq.map { legacyFormat =>
+ TimestampFormatter(
+ TimestampFormatter.defaultPattern(),
+ zoneId,
+ TimestampFormatter.defaultLocale,
+ legacyFormat,
+ isParsing = false)
+ } :+ TimestampFormatter.getFractionFormatter(zoneId)
+ formatters.foreach { formatter =>
+ assert(microsToInstant(formatter.parse("1000-01-01 01:02:03"))
+ .atZone(zoneId)
+ .toLocalDateTime === LocalDateTime.of(1000, 1, 1, 1, 2, 3))
+
+ assert(formatter.format(
+ LocalDateTime.of(1000, 1, 1, 1, 2, 3).atZone(zoneId).toInstant) ===
+ "1000-01-01 01:02:03")
+ assert(formatter.format(instantToMicros(
+ LocalDateTime.of(1000, 1, 1, 1, 2, 3)
+ .atZone(zoneId).toInstant)) === "1000-01-01 01:02:03")
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
test("parsing hour with various patterns") {
def createFormatter(pattern: String): TimestampFormatter = {
// Use `SIMPLE_DATE_FORMAT`, so that the legacy parser also fails with invalid value range.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org