HyukjinKwon commented on code in PR #59215:
URL: https://github.com/apache/spark/pull/59215#discussion_r4267068246
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/JavaTypeInference.scala:
##########
@@ -186,6 +188,45 @@ object JavaTypeInference {
throw ExecutionErrors.cannotFindEncoderForTypeError(t.toString)
}
+ /**
+ * Infer an [[AgnosticEncoder]] for a Java record (`java.lang.Record`).
Fields follow component
+ * declaration order and are read through the component accessors. Like the
JavaBean case,
+ * components are resolved through `encoderFor`, which carries `seenTypeSet`
and `typeVariables`
+ * into nested components.
+ */
+ private def encoderForJavaRecord[T](
+ recordClass: Class[T],
+ seenTypeSet: Set[Class[_]],
+ typeVariables: Map[TypeVariable[_], Type]): JavaRecordEncoder[T] = {
+ if (seenTypeSet.contains(recordClass)) {
+ throw
ExecutionErrors.cannotHaveCircularReferencesInBeanClassError(recordClass)
+ }
+
+ // Type parameters are erased, so they can only be resolved when the
enclosing type binds
+ // them (e.g. a `Box<String>` component), which puts them in
`typeVariables`.
+ val unboundTypeParams =
recordClass.getTypeParameters.filterNot(typeVariables.contains)
+ if (unboundTypeParams.nonEmpty) {
+ throw ExecutionErrors.genericRecordNotSupportedError(recordClass)
+ }
+
+ val fields = recordClass.getRecordComponents.map { component =>
+ val accessor = component.getAccessor
+ val encoder = encoderFor(component.getGenericType, seenTypeSet +
recordClass, typeVariables)
+ // The existence of `javax.annotation.Nonnull` means this component is
not nullable.
+ val hasNonNull = accessor.isAnnotationPresent(classOf[Nonnull])
Review Comment:
**Nit (P3):** `encoderForJavaRecord` reads `@Nonnull` only from the
accessor. Java copies a record-component annotation onto the accessor only when
the accessor is implicit. So for a record like `record R(@Nonnull String name)
{ public String name() { return name.strip(); } }`, `name` ends up nullable:
the schema reports it as nullable, and the deserializer skips the
`NOT_NULL_ASSERT_VIOLATION` check that the implicit-accessor form gets. I
checked this on JDK 25: for the explicit accessor,
`RecordComponent.isAnnotationPresent` is true but
`getAccessor().isAnnotationPresent` is false. Checking the component as well
fixes it and still honors an annotation placed only on the accessor:
```suggestion
val hasNonNull = component.isAnnotationPresent(classOf[Nonnull]) ||
accessor.isAnnotationPresent(classOf[Nonnull])
```
**Verification:**
- **Behavior:** Inference of a record whose @Nonnull component has an
explicitly declared accessor yields a non-nullable field, the same as the
implicit-accessor form.
- **Regression:** Decoding a row with null in that explicit-accessor
@Nonnull component fails with NOT_NULL_ASSERT_VIOLATION in codegen and
interpreted modes.
- **Compatibility:** Existing record nullability expectations, including the
implicit-accessor @Nonnull record and nullable components that round-trip null,
are unchanged.
--
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]
