bhollis-dbx opened a new pull request, #58253:
URL: https://github.com/apache/spark/pull/58253
### What changes were proposed in this pull request?
When `from_protobuf` is used with `unwrap.primitive.wrapper.types=true`, a
present well-known primitive wrapper
(`google.protobuf.{Bool,Int32,UInt32,Int64,UInt64,Float,Double,String,Bytes}Value`)
now unwraps to its inner scalar's default (0/""/false/empty-bytes) when the
inner value is unset, rather than to null. Null results only when the wrapper
field is absent. This is independent of `emit.default.values`.
The deserializer previously read the inner scalar through the same path used
for bare proto3 scalars (`getFieldValue`), which returns null for an
unset/default scalar unless `emit.default.values=true`. That conflated wrapper
*message presence* with the `emit.default.values` option (which governs bare
proto3 scalar defaults). The unwrap converters now read the inner value
directly via `DynamicMessage.getField`, which returns the scalar's default and
never null, matching proto3 wrapper semantics.
### Why are the changes needed?
- It matches the canonical proto3 JSON / wrapper mapping: wrapper presence
carries the value; absent -> null.
- It fixes an untested crash. For `repeated`/`map` fields of unwrapped
wrappers the container is non-nullable (`containsNull=false` /
`valueContainsNull=false`). A present-but-empty wrapper element used to
deserialize to null inside that non-null container, causing a downstream
`UnsafeWriter` NullPointerException. With the fix such elements become non-null
defaults, so the crash and the schema mismatch both disappear with no schema
change.
### Does this PR introduce _any_ user-facing change?
Yes. With `from_protobuf` and `unwrap.primitive.wrapper.types=true`, a
present wrapper whose inner value is the default now deserializes to that
default instead of null (and repeated/map wrappers with empty elements no
longer error). Absent wrappers still deserialize to null.
### How was this patch tested?
Updated the "test well known wrappers with emit defaults" expectations and
added a new test covering empty elements in a repeated wrapper (-> 0) and a map
wrapper value (-> ""). Ran the protobuf module suite: 94 tests pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
--
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]
