zhengruifeng commented on code in PR #58253:
URL: https://github.com/apache/spark/pull/58253#discussion_r4034519669
##########
connector/protobuf/src/main/scala/org/apache/spark/sql/protobuf/ProtobufDeserializer.scala:
##########
@@ -307,101 +307,51 @@ private[sql] class ProtobufDeserializer(
updater.set(ordinal, UTF8String.fromString(jsonStr))
// Handle well known wrapper types. We unpack the value field when the
desired
- // output type is a primitive (determined by the option in
[[ProtobufOptions]])
+ // output type is a primitive (determined by the option in
[[ProtobufOptions]]).
+ // A wrapper is unwrapped only when present: an absent singular wrapper
is nulled by the
+ // caller before reaching here, and container elements are always
present. So a present
+ // wrapper -- even an empty one -- carries a value, and unwrapWktValue
reads the inner
+ // scalar (its default when unset), never null, independent of
`emit.default.values`.
Review Comment:
**Non-blocking (P2):** The public option table still says
`emit.default.values` controls whether empty protobuf fields become `null` or a
type-specific zero, while this change deliberately makes a present unwrapped
wrapper return its scalar default independently of that option. Please update
the `emit.default.values` / `unwrap.primitive.wrapper.types` entries in
`docs/sql-data-sources-protobuf.md` to state that an absent singular wrapper
remains null and a present wrapper yields its scalar value or default.
Otherwise users cannot infer this upgrade's null-to-default result change from
the maintained reference.
--
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]
