GH-35866: [Go] Provide a copy in arrow.NestedType.Fields() implementations (#35867)

candiduslynx · web-flow · commit e0fd7ef8f16a · 2023-06-01T11:52:13.000-04:00
### Rationale for this change Data types should be left immutable after they've been constructed. ### What changes are included in this PR? Provide a copy of fields in the following implementations: * `*arrow.StructType.Fields()` * `*arrow.unionType.Fields()` ### Are these changes tested? `arrow.TestFieldsImmutability` was added to ensure the behavior. ### Are there any user-facing changes? Now the fields for structs & unions will be immutable. * Closes: #35866 Authored-by: candiduslynx <candiduslynx@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
diff --git a/go/arrow/datatype_nested.go b/go/arrow/datatype_nested.go
@@ -27,6 +27,9 @@ import (
 
 type NestedType interface {
 	DataType
+
+	// Fields method provides a copy of NestedType fields
+	// (so it can be safely mutated and will not result in updating the NestedType).
 	Fields() []Field
 }
 
@@ -288,7 +291,14 @@ func (t *StructType) String() string {
 	return o.String()
 }
 
-func (t *StructType) Fields() []Field   { return t.fields }
+// Fields method provides a copy of StructType fields
+// (so it can be safely mutated and will not result in updating the StructType).
+func (t *StructType) Fields() []Field {
+	fields := make([]Field, len(t.fields))
+	copy(fields, t.fields)
+	return fields
+}
+
 func (t *StructType) Field(i int) Field { return t.fields[i] }
 
 func (t *StructType) FieldByName(name string) (Field, bool) {
@@ -490,7 +500,14 @@ func (t *unionType) init(fields []Field, typeCodes []UnionTypeCode) {
 	}
 }
 
-func (t unionType) Fields() []Field            { return t.children }
+// Fields method provides a copy of union type fields
+// (so it can be safely mutated and will not result in updating the union type).
+func (t unionType) Fields() []Field {
+	fields := make([]Field, len(t.children))
+	copy(fields, t.children)
+	return fields
+}
+
 func (t unionType) TypeCodes() []UnionTypeCode { return t.typeCodes }
 func (t unionType) ChildIDs() []int            { return t.childIDs[:] }
 
diff --git a/go/arrow/datatype_nested_test.go b/go/arrow/datatype_nested_test.go
@@ -19,6 +19,9 @@ package arrow
 import (
 	"reflect"
 	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestListOf(t *testing.T) {
@@ -491,3 +494,50 @@ func TestMapOfWithMetadata(t *testing.T) {
 		})
 	}
 }
+
+func TestFieldsImmutability(t *testing.T) {
+	cases := []struct {
+		dt       NestedType
+		expected []Field
+	}{
+		{
+			dt:       ListOfField(Field{Name: "name", Type: PrimitiveTypes.Int64}),
+			expected: ListOfField(Field{Name: "name", Type: PrimitiveTypes.Int64}).Fields(),
+		},
+		{
+			dt:       LargeListOfField(Field{Name: "name", Type: PrimitiveTypes.Int64}),
+			expected: LargeListOfField(Field{Name: "name", Type: PrimitiveTypes.Int64}).Fields(),
+		},
+		{
+			dt:       FixedSizeListOfField(1, Field{Name: "name", Type: PrimitiveTypes.Int64}),
+			expected: FixedSizeListOfField(1, Field{Name: "name", Type: PrimitiveTypes.Int64}).Fields(),
+		},
+		{
+			dt:       MapOf(BinaryTypes.String, PrimitiveTypes.Int64),
+			expected: MapOf(BinaryTypes.String, PrimitiveTypes.Int64).Fields(),
+		},
+		{
+			dt:       StructOf(Field{Name: "name", Type: PrimitiveTypes.Int64}),
+			expected: StructOf(Field{Name: "name", Type: PrimitiveTypes.Int64}).Fields(),
+		},
+		{
+			dt:       RunEndEncodedOf(BinaryTypes.String, PrimitiveTypes.Int64),
+			expected: RunEndEncodedOf(BinaryTypes.String, PrimitiveTypes.Int64).Fields(),
+		},
+		{
+			dt:       UnionOf(DenseMode, []Field{{Name: "name", Type: PrimitiveTypes.Int64}}, []UnionTypeCode{0}),
+			expected: UnionOf(DenseMode, []Field{{Name: "name", Type: PrimitiveTypes.Int64}}, []UnionTypeCode{0}).Fields(),
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.dt.String(), func(t *testing.T) {
+			fields := tc.dt.Fields()
+			fields[0].Nullable = !fields[0].Nullable
+			fields[0].Name = uuid.NewString()
+			fields[0].Type = nil
+
+			assert.Equal(t, tc.expected, tc.dt.Fields())
+		})
+	}
+}
