HyukjinKwon commented on code in PR #59215:
URL: https://github.com/apache/spark/pull/59215#discussion_r4169827826
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/DeserializerBuildHelper.scala:
##########
@@ -448,6 +448,25 @@ object DeserializerBuildHelper {
exprs.Literal.create(null, dt),
NewInstance(cls, arguments, Nil, propagateNull = false, dt,
outerPointerGetter))
+ case AgnosticEncoders.JavaRecordEncoder(tag, fields) =>
+ val cls = tag.runtimeClass
+ val dt = ObjectType(cls)
+ val arguments = fields.zipWithIndex.map {
+ case (field, i) =>
+ val newTypePath = walkedTypePath.recordField(
+ field.enc.clsTag.runtimeClass.getName,
+ field.name)
+ val getter = addToPath(path, field.name, field.enc.dataType,
newTypePath)
+ expressionWithNullSafety(
+ createDeserializer(field.enc, getter, newTypePath),
+ field.enc.nullable,
Review Comment:
**Nit (P3):** This null-safety flag comes from the component encoder
(`field.enc.nullable`), as in the `ProductEncoder` arm above. But
`encoderForJavaRecord` also folds `@Nonnull` into `field.nullable`
(`encoder.nullable && !hasNonNull`), so for a `@Nonnull
String`/`Integer`/nested-record component `field.nullable` is `false` while
`field.enc.nullable` is `true`, and no `AssertNotNull` is generated. For
example, `spark.sql("SELECT CAST(NULL AS STRING) AS name, 1 AS count, 'n' AS
note").as(Encoders.record(NonnullRecord.class)).collect()` builds a
`NonnullRecord` with `name == null`, while a bean with a `@Nonnull` getter
fails with `NOT_NULL_ASSERT_VIOLATION` (the `JavaBeanEncoder` arm passes
`f.nullable`). The PR says record components follow the bean nullability rules
including `@Nonnull`, so passing `field.nullable` here would match that. A
round-trip test that decodes a null into a `@Nonnull` reference component would
also catch this; today only the encoder's nullable flag is as
serted.
**Verification:**
- **Regression:** Decoding an InternalRow with null in a @Nonnull String or
boxed component of a record, in both codegen and interpreted modes, raises a
NOT_NULL_ASSERT_VIOLATION failure.
- **Behavior:** Existing record round trips with nullable components
containing null still succeed.
--
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]
