MaxGekk commented on code in PR #56205:
URL: https://github.com/apache/spark/pull/56205#discussion_r3336108878
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala:
##########
@@ -789,6 +817,117 @@ trait SparkDateTimeUtils {
}
}
+ /**
+ * Trims and parses a given UTF8 string into a [[TimestampNanosVal]] (epoch
microseconds plus a
+ * sub-microsecond remainder in [0, 999]) for `TIMESTAMP_LTZ(precision)`
with `precision` in [7,
+ * 9]. Fractional digits beyond `precision` are truncated. The return type
is [[Option]] in
+ * order to distinguish between a valid zero value and null. Please refer to
+ * `parseTimestampString` for the allowed formats.
+ */
+ def stringToTimestampLTZNanos(
Review Comment:
Good call - done in d33e4e73dc4. I extracted two private helpers that do the
shared parse + `java.time` construction and return the intermediate value:
- `parseTimestampToInstant(s, timeZoneId): Instant` - shared by
`stringToTimestamp` (micros) and `stringToTimestampLTZNanos` (nanos)
- `parseTimestampToLocalDateTime(s, allowTimeZone): LocalDateTime` - shared
by `stringToTimestampWithoutTimeZone` (micros) and `stringToTimestampNTZNanos`
(nanos)
Each public entry point now keeps only its cheap, type-specific tail inlined
(`instantToMicros`/`localDateTimeToMicros` vs
`instantToTimestampNanos`/`localDateTimeToTimestampNanos`).
One deliberate choice worth flagging: the helpers return `null` on an
unparseable string rather than `Option`. Both `stringToTimestamp*` families are
cast hot paths (and we plan to wire the nanos variants into casts shortly), so
I wanted the dedup to add zero allocation - a shared core returning
`Option`/`.map` or a higher-order function would materialize an extra
`Option`/closure per call, and making the richer `TimestampNanosVal` the base
would force the micro path to allocate it (plus run the precision truncation)
only to discard the sub-micro digits. The `null` sentinel keeps the helper
small enough to inline and leaves the micro path allocation-identical to
before. The choice and the caller null-checks are documented in the helper
scaladoc.
The helpers carry the full fraction (including the sub-microsecond digits)
into the `java.time` value; that's behavior-preserving for the micro path since
`instantToMicros`/`localDateTimeToMicros` floor the sub-micro part - covered by
the existing micro-path regression test.
Verified: `TimestampNanosParseSuite` (20), `DateTimeUtilsSuite` (72), and
`CastWithAnsiOnSuite`/`CastWithAnsiOffSuite`/`TryCastSuite` (307) all pass,
scalastyle clean.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
