From 2ec798608e251b0763eae564716ad76b6f678a0f Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Thu, 26 Jan 2023 17:21:42 -0400 Subject: [PATCH 1/3] fix!: empty slice instead of nil slice for primitive repeated fields --- bigquery/integration_test.go | 32 ++++++++++++++++++++++++++++++++ bigquery/value.go | 3 +++ 2 files changed, 35 insertions(+) diff --git a/bigquery/integration_test.go b/bigquery/integration_test.go index 81c1b123444c..268c6d9f1c12 100644 --- a/bigquery/integration_test.go +++ b/bigquery/integration_test.go @@ -24,6 +24,7 @@ import ( "math/big" "net/http" "os" + "reflect" "sort" "strings" "testing" @@ -2242,6 +2243,37 @@ func TestIntegration_QueryParameters(t *testing.T) { } } +func TestIntegration_QueryEmptyArrays(t *testing.T) { + if client == nil { + t.Skip("Integration tests skipped") + } + ctx := context.Background() + + q := client.Query("SELECT ARRAY[] as a, ARRAY>[] as b") + it, err := q.Read(ctx) + if err != nil { + t.Fatal(err) + } + for { + vals := map[string]Value{} + if err := it.Next(&vals); err != nil { + if errors.Is(err, iterator.Done) { + break + } + } + + valueOfA := reflect.ValueOf(vals["a"]) + if testutil.Equal(vals["a"], nil) || valueOfA.IsNil() { + t.Fatalf("expected empty string array to not return nil, but found %v %v %T", valueOfA, vals["a"], vals["a"]) + } + + valueOfB := reflect.ValueOf(vals["b"]) + if testutil.Equal(vals["b"], nil) || valueOfB.IsNil() { + t.Fatalf("expected empty struct array to not return nil, but found %v %v %T", valueOfB, vals["b"], vals["b"]) + } + } +} + // This test can be merged with the TestIntegration_QueryParameters as soon as support for explicit typed query parameter lands. // To test timestamps with different formats, we need to be able to specify the type explicitly. func TestIntegration_TimestampFormat(t *testing.T) { diff --git a/bigquery/value.go b/bigquery/value.go index d1dffa91ca7f..0187c9861606 100644 --- a/bigquery/value.go +++ b/bigquery/value.go @@ -83,6 +83,9 @@ func loadMap(m map[string]Value, vals []Value, s Schema) { } v = vs } + if reflect.ValueOf(v).IsNil() && f.Repeated { + v = []Value{} + } m[f.Name] = v } From 7a519ef70e5c93d6a19a836a2b2cce2b7164a87a Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Fri, 27 Jan 2023 10:27:18 -0400 Subject: [PATCH 2/3] fix(bigquery): check for nil value before reflect.ValueOf.IsNil --- bigquery/value.go | 2 +- bigquery/value_test.go | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/bigquery/value.go b/bigquery/value.go index 0187c9861606..f31599961e41 100644 --- a/bigquery/value.go +++ b/bigquery/value.go @@ -83,7 +83,7 @@ func loadMap(m map[string]Value, vals []Value, s Schema) { } v = vs } - if reflect.ValueOf(v).IsNil() && f.Repeated { + if f.Repeated && (v == nil || reflect.ValueOf(v).IsNil()) { v = []Value{} } diff --git a/bigquery/value_test.go b/bigquery/value_test.go index 6b051366a6f6..9ce9a0c12edc 100644 --- a/bigquery/value_test.go +++ b/bigquery/value_test.go @@ -851,10 +851,12 @@ func TestValueMap(t *testing.T) { {Name: "i", Type: IntegerFieldType}, {Name: "f", Type: FloatFieldType}, {Name: "b", Type: BooleanFieldType}, - {Name: "n", Type: RecordFieldType, Schema: ns}, + {Name: "sn", Type: StringFieldType, Repeated: true}, + {Name: "r", Type: RecordFieldType, Schema: ns}, {Name: "rn", Type: RecordFieldType, Schema: ns, Repeated: true}, } in := []Value{"x", 7, 3.14, true, + []Value{"a", "b"}, []Value{1, 2}, []Value{[]Value{3, 4}, []Value{5, 6}}, } @@ -863,11 +865,12 @@ func TestValueMap(t *testing.T) { t.Fatal(err) } want := map[string]Value{ - "s": "x", - "i": 7, - "f": 3.14, - "b": true, - "n": map[string]Value{"x": 1, "y": 2}, + "s": "x", + "i": 7, + "f": 3.14, + "b": true, + "sn": []Value{"a", "b"}, + "r": map[string]Value{"x": 1, "y": 2}, "rn": []Value{ map[string]Value{"x": 3, "y": 4}, map[string]Value{"x": 5, "y": 6}, @@ -883,8 +886,9 @@ func TestValueMap(t *testing.T) { "i": nil, "f": nil, "b": nil, - "n": nil, - "rn": nil, + "sn": []Value{}, + "r": nil, + "rn": []Value{}, } var vm2 valueMap if err := vm2.Load(in, schema); err != nil { From f1875c5efb2958dc49de1afaf27bec05326c6530 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Thu, 16 May 2024 13:23:37 -0400 Subject: [PATCH 3/3] docs: add more details around empty vs null arrays --- bigquery/iterator.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bigquery/iterator.go b/bigquery/iterator.go index 9d177d1b829c..942be4205d07 100644 --- a/bigquery/iterator.go +++ b/bigquery/iterator.go @@ -140,8 +140,12 @@ type pageFetcher func(ctx context.Context, _ *rowSource, _ Schema, startIndex ui // See https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#numeric-type // for more on NUMERIC. // -// A repeated field corresponds to a slice or array of the element type. A STRUCT -// type (RECORD or nested schema) corresponds to a nested struct or struct pointer. +// A repeated field corresponds to a slice or array of the element type. BigQuery translates +// NULL arrays into an empty array, so we follow that behavior. +// See https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#array_nulls +// for more about NULL and empty arrays. +// +// A STRUCT type (RECORD or nested schema) corresponds to a nested struct or struct pointer. // All calls to Next on the same iterator must use the same struct type. // // It is an error to attempt to read a BigQuery NULL value into a struct field,