MaxGekk commented on code in PR #56449:
URL: https://github.com/apache/spark/pull/56449#discussion_r3408539033
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/types/ops/TimestampNanosTypeOps.scala:
##########
@@ -130,11 +131,14 @@ case class TimestampNTZNanosTypeOps(override val t:
TimestampNTZNanosType)
* The TimestampLTZNanosType with precision information
* @since 4.3.0
*/
-// Server-side TypeOps is used only for physical type, literals, row
accessors, and serde - never
-// for rendering (cast-to-string flows through TypeApiOps.apply). So the
rendering zone is unused
-// here; pass UTC rather than reading the session config on every construction.
+// LTZ is zone-aware, so the inherited renderer needs a zone. The server-side
TypeOps has none of
+// its own (rendering normally flows through TypeApiOps.apply, which threads
the cast's resolved
+// zone), so default to the session-local time zone. It is passed by-name and
the parent builds
+// its formatter lazily, so the config is read only if a value is actually
rendered through this
+// server-side ops - never on the physical-type / literal / row-accessor /
serde paths.
case class TimestampLTZNanosTypeOps(override val t: TimestampLTZNanosType)
- extends TimestampLTZNanosTypeApiOps(t, ZoneOffset.UTC) with
TimestampNanosTypeOps {
+ extends TimestampLTZNanosTypeApiOps(t,
DateTimeUtils.getZoneId(SQLConf.get.sessionLocalTimeZone))
Review Comment:
This is the behavior the follow-up commit adds -- server-side
`TimestampLTZNanosTypeOps` now renders LTZ in the session-local zone rather
than UTC -- but nothing in the test suite exercises it.
`TimestampLTZNanosTypeOps(` appears only here and at the factory registration
(`TypeOps.scala:252`); `TimestampNanosTypeOpsSuite` covers only `new
TimestampLTZNanosTypeApiOps(t, <explicit zone>)` and the `TypeApiOps(dt)`
zone-less lookup, so a regression of this line back to a fixed zone would pass
CI silently.
Consider a small test under a non-UTC session zone, e.g.:
```scala
withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> "America/Los_Angeles") {
val ops = TimestampLTZNanosTypeOps(TimestampLTZNanosType(p))
// assert ops.format(v) renders the wall clock in the session zone
}
```
Non-blocking -- the server-side render path has no production caller today,
so this guards the documented contract rather than fixing a live bug.
--
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]
