zeroshade opened a new pull request, #918:
URL: https://github.com/apache/arrow-go/pull/918

   ### Rationale for this change
   
   This is the inner-nullability parity work that was previously bundled into 
#558 (the `TimestampWithOffset` canonical extension type). #558 has now been 
reduced to just the extension type, and this PR carries the comparison and 
JSON-marshaling changes on their own so they can be reviewed independently and 
more carefully.
   
   ### What changes are included in this PR?
   
   - `arrow/array/compare.go`: thread each field's `Nullable` flag through 
`Equal`/`RecordEqual`/`TableEqual`/`ChunkedEqual` (and the `Approx` variants) 
so that non-nullable fields ignore validity-bitmap differences; add a 
`WithNullable` functional option; compare record/table fields against the 
right-hand schema (not the left against itself) and ignore field metadata 
during record/table comparison.
   - `arrow.Array` interface: keep the single-argument `GetOneForMarshal(i 
int)` and move the field-local behavior to a new optional `NullableMarshaler` 
interface with `GetOneForMarshalNullable(i int, nullable bool)`, so external 
callers and custom `Array` implementations are not broken. Containers (struct, 
union, dictionary, run-end-encoded, extension) and `RecordToJSON` propagate 
each field's nullability through an unexported helper that falls back to 
`GetOneForMarshal` when the optional interface is not implemented.
   - Struct/record: honor `field.Nullable` when serializing to JSON, and handle 
explicit `null` for a non-nullable field when reading.
   - `internal/json`: add a helper to detect a `null` message.
   - Test fixtures updated to declare inner struct field nullability explicitly.
   
   ### Relationship to #833
   
   #833 pursues the same goal with a different strategy: it drops the 
comparison leniency (`WithNullable`/`equalOption`) in favor of schema 
validation in `Validate()` to match Arrow C++, and checks `IsValid()` in the 
struct/record builders rather than introducing `GetOneForMarshalNullable()`. 
This PR preserves the earlier comparison-based approach. Reviewers should 
decide which direction to land; opening as a draft pending that decision.
   
   ### Are these changes tested?
   
   Yes. `go test ./arrow/array/ ./arrow/extensions/ ./arrow/internal/arrjson/ 
./arrow/compute/ ./arrow/ipc/...` pass.
   
   ### Are there any user-facing changes?
   
   Yes: comparison functions gain a `WithNullable` option and the array 
marshaling path gains an optional `NullableMarshaler` interface. The existing 
`GetOneForMarshal(i int)` signature is preserved.
   


-- 
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]

Reply via email to