davidm-db commented on code in PR #56266:
URL: https://github.com/apache/spark/pull/56266#discussion_r3348000876
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/EncoderUtils.scala:
##########
@@ -119,16 +117,16 @@ object EncoderUtils {
}
}
- @scala.annotation.tailrec
- def javaBoxedType(dt: DataType): Class[_] = dt match {
+ def javaBoxedType(dt: DataType): Class[_] =
+ TypeOps(dt).map(_.getBoxedJavaClass).getOrElse(javaBoxedTypeDefault(dt))
Review Comment:
Question (non-blocking): is `getBoxedJavaClass` reachable for the framework
types today? `javaBoxedType` has two callers - `ValidateExternalType.checkType`
and `MapObjects.elementClassTag` - and for non-native encoders both receive the
external `ObjectType(...)` (e.g. `ObjectType(LocalTime)` /
`ObjectType(LocalDateTime)`), not the raw `TimestampNTZNanosType` / `TimeType`;
`MapObjects`'s boxed-element branch only fires for native catalyst element
types. So `TypeOps(dt).map(_.getBoxedJavaClass)` appears to return `Some` only
for types that never reach `javaBoxedType` with their literal form. If that's
right, the hook plus this wiring is effectively defensive for now - which is
fine and consistent with centralizing the dispatch (the explicit nanos arms it
replaces were equally unreached) - I just want to confirm it's intentional
rather than closing a live gap. The hook is still the right shape if it ever
does become reachable, since `getJavaClass` returns the primitive class for Long
-backed types.
--
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]
