serramatutu commented on code in PR #833:
URL: https://github.com/apache/arrow-go/pull/833#discussion_r3864225637


##########
arrow/array/record.go:
##########
@@ -434,49 +436,74 @@ func (b *RecordBuilder) UnmarshalOne(dec *json.Decoder) 
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 {
+                       b.Resize(-1)

Review Comment:
   I realized a new `builderCheckpoint` waas just introduced. Will use that to 
rollback instead.



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