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

[ze...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new e0fd7ef8f1 GH-35866: [Go] Provide a copy in `arrow.NestedType.Fields()` implementations (#35867)
e0fd7ef8f1 is described below

commit e0fd7ef8f16a913913bba706c396f424fba1583a
Author: Alex Shcherbakov <ca...@users.noreply.github.com>
AuthorDate: Thu Jun 1 18:52:13 2023 +0300

    GH-35866: [Go] Provide a copy in `arrow.NestedType.Fields()` implementations (#35867)
    
    ### 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 <ca...@gmail.com>
    Signed-off-by: Matt Topol <zo...@gmail.com>
---
 go/arrow/datatype_nested.go      | 21 +++++++++++++++--
 go/arrow/datatype_nested_test.go | 50 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/go/arrow/datatype_nested.go b/go/arrow/datatype_nested.go
index de544251cc..368527b4b2 100644
--- 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
index 0f4203b0da..15d7b2e0e3 100644
--- 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())
+		})
+	}
+}