davidm-db commented on code in PR #56439:
URL: https://github.com/apache/spark/pull/56439#discussion_r3405603106
##########
sql/api/src/main/scala/org/apache/spark/sql/types/ops/TypeApiOps.scala:
##########
@@ -187,23 +186,19 @@ object TypeApiOps {
def apply(
dt: DataType,
zoneId: => ZoneId =
SparkDateTimeUtils.getZoneId(SqlApiConf.get.sessionLocalTimeZone))
- : Option[TypeApiOps] = {
- if (!SqlApiConf.get.typesFrameworkEnabled) return None
- dt match {
- case tt: TimeType => Some(new TimeTypeApiOps(tt))
- case t: TimestampNTZNanosType => Some(new TimestampNTZNanosTypeApiOps(t))
- case t: TimestampLTZNanosType => Some(new TimestampLTZNanosTypeApiOps(t,
zoneId))
- // Add new types here - single registration point
- case _ => None
- }
+ : Option[TypeApiOps] = dt match {
Review Comment:
Good catch. That comment documents the legacy `Row.toJsonDefault` TIME arm,
which this flag removal makes unreachable -- `TypeApiOps` now always resolves
for TIME, so Row JSON renders through `formatExternal` and never falls back
here. It's one of the wider legacy TIME fallbacks I discussed with Max in the
`SerializerBuildHelper` thread (`HiveResult.toHiveString`, the Connect
converters, ...); I'm keeping this PR flag-specific and removing all of them
together in a TIME-specific follow-up, where I'd update this comment alongside
deleting the dead arm.
--
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]
