Skip to content

Commit e2daf37

Browse files
committed
Add stricter tests to null JSON in record and struct
1 parent 08f25c8 commit e2daf37

2 files changed

Lines changed: 63 additions & 33 deletions

File tree

arrow/array/record_test.go

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package array_test
1818

1919
import (
20+
"bytes"
2021
"fmt"
2122
"reflect"
2223
"strings"
@@ -25,6 +26,7 @@ import (
2526
"github.com/apache/arrow-go/v18/arrow"
2627
"github.com/apache/arrow-go/v18/arrow/array"
2728
"github.com/apache/arrow-go/v18/arrow/memory"
29+
"github.com/apache/arrow-go/v18/internal/json"
2830
"github.com/stretchr/testify/assert"
2931
)
3032

@@ -485,9 +487,9 @@ func TestRecordBuilder(t *testing.T) {
485487
mapDt.SetItemNullable(false)
486488
schema := arrow.NewSchema(
487489
[]arrow.Field{
488-
{Name: "f1-i32", Type: arrow.PrimitiveTypes.Int32},
489-
{Name: "f2-f64", Type: arrow.PrimitiveTypes.Float64},
490-
{Name: "map", Type: mapDt},
490+
{Name: "f1-i32", Type: arrow.PrimitiveTypes.Int32, Nullable: true},
491+
{Name: "f2-f64-notnull", Type: arrow.PrimitiveTypes.Float64, Nullable: false},
492+
{Name: "map", Type: mapDt, Nullable: true},
491493
},
492494
nil,
493495
)
@@ -498,11 +500,14 @@ func TestRecordBuilder(t *testing.T) {
498500
b.Retain()
499501
b.Release()
500502

501-
b.Field(0).(*array.Int32Builder).AppendValues([]int32{1, 2, 3}, nil)
503+
b.Field(0).(*array.Int32Builder).AppendNull()
504+
b.Field(0).(*array.Int32Builder).AppendValues([]int32{2, 3}, nil)
502505
b.Field(0).(*array.Int32Builder).AppendValues([]int32{4, 5}, nil)
503-
b.Field(1).(*array.Float64Builder).AppendValues([]float64{1, 2, 3, 4, 5}, nil)
506+
507+
b.Field(1).(*array.Float64Builder).AppendValues([]float64{1.1, 2.2, 3.3, 4.4, 5.5}, nil)
508+
504509
mb := b.Field(2).(*array.MapBuilder)
505-
for i := 0; i < 5; i++ {
510+
for i := range 5 {
506511
mb.Append(true)
507512

508513
if i%3 == 0 {
@@ -511,14 +516,23 @@ func TestRecordBuilder(t *testing.T) {
511516
}
512517
}
513518

519+
err := b.UnmarshalJSON([]byte(`{"f1-i32": null, "f2-f64-notnull": null, "map": null}`))
520+
assert.Contains(t, err.Error(), "field 'f2-f64-notnull' is non-nullable but got null")
521+
522+
err = b.UnmarshalJSON([]byte(`{"f1-i32": null, "map": null}`))
523+
assert.Contains(t, err.Error(), "field 'f2-f64-notnull' is required but no value was given")
524+
525+
err = b.UnmarshalJSON([]byte(`{"f1-i32": 6, "f2-f64-notnull": 6.6, "map": [{"key": "4": "value": "d"}]}`))
526+
assert.NoError(t, err)
527+
514528
rec := b.NewRecordBatch()
515529
defer rec.Release()
516530

517531
if got, want := rec.Schema(), schema; !got.Equal(want) {
518532
t.Fatalf("invalid schema: got=%#v, want=%#v", got, want)
519533
}
520534

521-
if got, want := rec.NumRows(), int64(5); got != want {
535+
if got, want := rec.NumRows(), int64(6); got != want {
522536
t.Fatalf("invalid number of rows: got=%d, want=%d", got, want)
523537
}
524538
if got, want := rec.NumCols(), int64(3); got != want {
@@ -527,9 +541,27 @@ func TestRecordBuilder(t *testing.T) {
527541
if got, want := rec.ColumnName(0), schema.Field(0).Name; got != want {
528542
t.Fatalf("invalid column name: got=%q, want=%q", got, want)
529543
}
530-
if got, want := rec.Column(2).String(), `[{["0" "2" "3"] ["a" "b" "c"]} {[] []} {[] []} {["3" "2" "3"] ["a" "b" "c"]} {[] []}]`; got != want {
531-
t.Fatalf("invalid column name: got=%q, want=%q", got, want)
544+
545+
if got, want := rec.Column(0).String(), `[(null) 2 3 4 5 6]`; got != want {
546+
t.Fatalf("invalid column values: got=%q, want=%q", got, want)
547+
}
548+
if got, want := rec.Column(1).String(), `[1.1 2.2 3.3 4.4 5.5 6.6]`; got != want {
549+
t.Fatalf("invalid column values: got=%q, want=%q", got, want)
532550
}
551+
if got, want := rec.Column(2).String(), `[{["0" "2" "3"] ["a" "b" "c"]} {[] []} {[] []} {["3" "2" "3"] ["a" "b" "c"]} {[] []} {["4"] ["d"]}]`; got != want {
552+
t.Fatalf("invalid column values: got=%q, want=%q", got, want)
553+
}
554+
555+
// roundtripping from JSON with array.FromJSON should work
556+
arr := array.RecordToStructArray(rec)
557+
defer arr.Release()
558+
jsonStr, err := json.Marshal(arr)
559+
assert.NoError(t, err)
560+
561+
roundtripped, _, err := array.FromJSON(mem, arr.DataType(), bytes.NewReader(jsonStr))
562+
defer roundtripped.Release()
563+
assert.NoError(t, err)
564+
assert.Truef(t, array.Equal(arr, roundtripped), "JSON round trip returns different array: got=%q, want=%d", arr, roundtripped)
533565
}
534566

535567
func TestRecordBuilderResize(t *testing.T) {

arrow/array/struct_test.go

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package array_test
1818

1919
import (
20+
"errors"
21+
"fmt"
2022
"reflect"
2123
"testing"
2224

@@ -472,52 +474,48 @@ func TestStructArrayUnmarshalJSONMissingFields(t *testing.T) {
472474
name string
473475
jsonInput string
474476
want string
475-
panic bool
477+
panicErr error
476478
}{
477479
{
478480
name: "missing required field",
479481
jsonInput: `[{"f2": 3, "f3": {"f3_1": "test"}}]`,
480-
panic: true,
482+
panicErr: errors.New("arrow/array: index out of range"),
481483
want: "",
482484
},
483485
{
484486
name: "missing optional fields",
485487
jsonInput: `[{"f2": 3, "f3": {"f3_3": "test"}}]`,
486-
panic: false,
488+
panicErr: nil,
487489
want: `{[(null)] [3] {[(null)] [(null)] ["test"]}}`,
488490
},
491+
{
492+
name: "explicit null in required field",
493+
jsonInput: `[{"f2": 3, "f3": {"f3_3": null}}]`,
494+
panicErr: errors.New("field 'f3_3' is non-nullable but got null"),
495+
want: "",
496+
},
489497
}
490498

491499
for _, tc := range tests {
492500
t.Run(
493501
tc.name, func(t *testing.T) {
494-
495-
var val bool
496-
497502
sb := array.NewStructBuilder(pool, dtype)
498503
defer sb.Release()
499504

500-
if tc.panic {
501-
defer func() {
502-
e := recover()
503-
if e == nil {
504-
t.Fatalf("this should have panicked, but did not; slice value %v", val)
505-
}
506-
if got, want := e.(string), "arrow/array: index out of range"; got != want {
507-
t.Fatalf("invalid error. got=%q, want=%q", got, want)
508-
}
509-
}()
510-
} else {
511-
defer func() {
512-
if e := recover(); e != nil {
513-
t.Fatalf("unexpected panic: %v", e)
514-
}
515-
}()
516-
}
505+
defer func() {
506+
e := recover()
507+
if e == nil && tc.panicErr != nil {
508+
t.Fatalf("did not panic, expected panic: %v", tc.panicErr)
509+
} else if e != nil && tc.panicErr == nil {
510+
t.Fatalf("unexpected panic: %v", e)
511+
} else if e != nil && tc.panicErr != nil && fmt.Errorf("%s", e).Error() != tc.panicErr.Error() {
512+
t.Fatalf("invalid error. got=%v, want=%v", e, tc.panicErr.Error())
513+
}
514+
}()
517515

518516
err := sb.UnmarshalJSON([]byte(tc.jsonInput))
519517
if err != nil {
520-
t.Fatal(err)
518+
panic(err)
521519
}
522520

523521
arr := sb.NewArray().(*array.Struct)

0 commit comments

Comments
 (0)