serramatutu commented on code in PR #833:
URL: https://github.com/apache/arrow-go/pull/833#discussion_r3864335360
##########
arrow/array/util.go:
##########
@@ -296,7 +296,11 @@ func RecordToJSON(rec arrow.RecordBatch, w io.Writer)
error {
cols := make(map[string]interface{})
for i := 0; int64(i) < rec.NumRows(); i++ {
for j, c := range rec.Columns() {
- cols[fields[j].Name] = c.GetOneForMarshal(i)
+ if rec.Schema().Field(j).Nullable && c.IsNull(i) {
Review Comment:
Per this: https://github.com/serramatutu/arrow-internal-nulls
In C++, it looks like the general approach is "validate all data in", then
assume it's OK/consistent when "writing out". I.e it is an invariant that the
data is correct when writing out.
If the user has messed around with the data:
- by tweaking memory directly: they need to ensure their code respects
invariants OR call ValidateFull
- by using a public API: the API will always leave the array in a consistent
internal state
This is my rationale for calling `GetOneForMarshal()` here: we assume the
data is correct, if it's not then it's UB.
If we're very pedantic about this, we should just revert this change and use
`c.GetOneForMarshal(i)` as we should assume `IsNull(i)` will always return
false if the data is consistent with the schema. This is what C++ does
actually, so here we're doing even more validation by checking
`rec.Schema().Field(j).Nullable`.
Sources:
- `JsonWriter::WriteArray` uses an `ArrayWriter`:
https://github.com/apache/arrow/blob/b12755a1cc41ee5e9b1874ece3bcc92d6d3e3e78/cpp/src/arrow/integration/json_internal.cc#L2079
- `ArrayWriter::WriteDataValues` does not check for the schema at all, it
just calls `IsValid()` directly:
https://github.com/apache/arrow/blob/b12755a1cc41ee5e9b1874ece3bcc92d6d3e3e78/cpp/src/arrow/integration/json_internal.cc#L505
--
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]
