serramatutu commented on code in PR #833:
URL: https://github.com/apache/arrow-go/pull/833#discussion_r3873161854
##########
arrow/array/record.go:
##########
@@ -599,49 +601,73 @@ func (b *RecordBuilder) unmarshalOne(dec *json.Decoder)
(err error) {
return fmt.Errorf("record should start with '{', not %s", t)
}
- keylist := make(map[string]bool)
+ // consume one row checking for duplicates and nulls
+ keylist := make(map[string]json.RawMessage)
for dec.More() {
keyTok, err := dec.Token()
if err != nil {
return err
}
key := keyTok.(string)
- if keylist[key] {
+ if _, ok := keylist[key]; ok {
return fmt.Errorf("key %s shows up twice in row to be
decoded", key)
}
- keylist[key] = true
+
+ var val json.RawMessage
+ if err := dec.Decode(&val); err != nil {
+ return err
+ }
indices := b.schema.FieldIndices(key)
if len(indices) == 0 {
- var extra interface{}
- if err := dec.Decode(&extra); err != nil {
- return err
- }
continue
}
- if err := b.fields[indices[0]].UnmarshalOne(dec); err != nil {
- return err
+ idx := indices[0]
+
+ if bytes.Equal(val, []byte("null")) &&
!b.schema.Field(idx).Nullable {
+ return fmt.Errorf("field '%s' is non-nullable but got
null", key)
}
+
+ keylist[key] = val
}
// consume the closing '}'
if _, err := dec.Token(); err != nil {
return err
}
+ // check that all non-nullable fields were specified
for i := 0; i < b.schema.NumFields(); i++ {
- if !keylist[b.schema.Field(i).Name] {
+ f := b.schema.Field(i)
+ if _, ok := keylist[f.Name]; !ok && !f.Nullable {
+ return fmt.Errorf("field '%s' is required but no value
was given", f.Name)
+ }
+ }
+
+ // At this point we know there are no integrity errors, so append
values to the
+ // field builders in schema order.
+ for i := 0; i < b.schema.NumFields(); i++ {
+ val, ok := keylist[b.schema.Field(i).Name]
+ if !ok {
b.fields[i].AppendNull()
+ continue
+ }
+
+ valDec := json.NewDecoder(bytes.NewReader(val))
+ valDec.UseNumber()
+ if err := b.fields[i].UnmarshalOne(valDec); err != nil {
Review Comment:
I was still fixing this. I just fixed it for list, map, union and REE, and
added tests for all of those. I also made all of them have individual
checkpointing so if users use e.g a standalone `ListBuilder` with a
non-nullable field, it'll still keep a consistent state.
Note that I had to add a new `NewListBuilderWithField` (also for list view
and large lists) so that the builder can know the schema it's trying to build.
--
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]
