davidm-db commented on code in PR #56513: URL: https://github.com/apache/spark/pull/56513#discussion_r3413932548
########## sql/api/src/main/scala/org/apache/spark/sql/types/ops/TimestampNanosTypeApiOps.scala: ########## @@ -58,16 +58,14 @@ abstract class TimestampNanosTypeApiOps extends TypeApiOps with DataTypeErrorsBa // ==================== String Formatting ==================== - // Row JSON (Row.json / Row.prettyJson) holds the external Row value (java.time.Instant for LTZ, - // java.time.LocalDateTime for NTZ); each subclass overrides the single-arg formatExternal to - // render it through the same formatter as its zone-aware cast-to-string, so Row JSON shows the - // nanosecond value rather than silently truncating to microseconds via the legacy path. - - // The Hive result path (HiveResult.toHiveString) renders nanosecond timestamps through its own - // zone-aware default formatter, so return None here to fall through to it rather than to the - // subclass single-arg rendering. This is a temporary split until nanos external rendering is - // unified across the zone-less (Row JSON) and zone-aware (Hive) paths. - override def formatExternal(value: Any, nested: Boolean): Option[String] = None + // Both external-value consumers share the single-arg formatExternal each subclass overrides: + // - Row JSON (Row.json / Row.prettyJson) holds the external Row value (java.time.Instant for + // LTZ, java.time.LocalDateTime for NTZ). + // - The Hive result path (HiveResult.toHiveString) calls the two-arg overload, whose default + // delegates to the single-arg renderer; `nested` does not affect timestamp formatting. + // Each subclass renders the external value through the same formatter as its zone-aware + // cast-to-string, so both paths show the nanosecond value rather than silently truncating to + // microseconds via the legacy path. Review Comment: do we even need this comment now - it's free floating and to a random person reading the code it wouldn't refer to anything concrete, especially considering that we removed the actual override? -- 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]
