MaxGekk commented on code in PR #56449:
URL: https://github.com/apache/spark/pull/56449#discussion_r3403422523
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/types/ops/TypeOps.scala:
##########
@@ -37,6 +38,12 @@ import org.apache.spark.sql.types.{DataType,
TimestampLTZNanosType, TimestampNTZ
* implemented by every type. Optional methods (serialization, Arrow writer)
return Option and
* default to None - types implement them as they expand their integration
coverage.
*
+ * Extends [[TypeApiOps]] (the api-side trait) so that callers holding an
`Option[TypeOps]` can
+ * invoke client-side methods (`format`, `toSQLValue`, `getEncoder`, ...)
directly via `.map`,
Review Comment:
The advertised usage is unsafe for the LTZ nanos type.
`TimestampLTZNanosTypeOps` builds its `TypeApiOps` parent with a hardcoded
`ZoneOffset.UTC` (TimestampNanosTypeOps.scala:136-137), relying on the
invariant stated right above it: server-side ops are "never used for
rendering". LTZ `format`/`formatExternal` render through a formatter built from
that zone (TimestampNanosTypeApiOps.scala:138-147). This doc now invites
exactly those calls on `TypeOps` values, so a caller following it gets silent
UTC rendering instead of session-zone rendering for TIMESTAMP_LTZ nanos.
I'd fix it by making the zone real but lazy: change
`TimestampLTZNanosTypeApiOps`'s `zoneId` to a by-name parameter (`zoneId: =>
ZoneId` — `TypeApiOps.apply` already takes it by-name for the same
construction-cost reason) and have the catalyst ops pass the session-zone
lookup instead of UTC. The serde-only path never forces it, and rendering
through `TypeOps` becomes correct. If you'd rather keep rendering off the
server path entirely, then this doc should say so — scope it to the
zone-independent operations and restate the no-rendering invariant here.
Nit in the same sentence: "runtime narrow" -> "runtime narrowing".
--
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]
