Skip to content

Commit

Permalink
[SPARK-48042][SQL] Use a timestamp formatter with timezone at class l…
Browse files Browse the repository at this point in the history
…evel instead of making copies at method level

### What changes were proposed in this pull request?

This PR creates a timestamp formatter with the timezone directly for formatting. Previously, we called `withZone` for every value in the `format` function. Because the original `zoneId` in the formatter is null and never equals the one we pass in, it creates new copies of the formatter over and over.

```java
...
     *
     * param zone  the new override zone, null if no override
     * return a formatter based on this formatter with the requested override zone, not null
     */
    public DateTimeFormatter withZone(ZoneId zone) {
        if (Objects.equals(this.zone, zone)) {
            return this;
        }
        return new DateTimeFormatter(printerParser, locale, decimalStyle, resolverStyle, resolverFields, chrono, zone);
    }
```

### Why are the changes needed?

improvement
### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

- Existing tests
- I also ran the DateTimeBenchmark result locally, there's no performance gain at least for these cases.

### Was this patch authored or co-authored using generative AI tooling?
no

Closes apache#46282 from yaooqinn/SPARK-48042.

Authored-by: Kent Yao <yao@apache.org>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
  • Loading branch information
yaooqinn authored and dongjoon-hyun committed Apr 29, 2024
1 parent c35a21e commit c9ed9df
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ class Iso8601TimestampFormatter(
protected lazy val formatter: DateTimeFormatter =
getOrCreateFormatter(pattern, locale, isParsing)

@transient
private lazy val zonedFormatter: DateTimeFormatter = formatter.withZone(zoneId)

@transient
protected lazy val legacyFormatter = TimestampFormatter.getLegacyFormatter(
pattern, zoneId, locale, legacyFormat)
Expand Down Expand Up @@ -231,7 +234,7 @@ class Iso8601TimestampFormatter(

override def format(instant: Instant): String = {
try {
formatter.withZone(zoneId).format(instant)
zonedFormatter.format(instant)
} catch checkFormattedDiff(toJavaTimestamp(instantToMicros(instant)),
(t: Timestamp) => format(t))
}
Expand Down

0 comments on commit c9ed9df

Please sign in to comment.