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

[zeroshade]

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 <candidusl...@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 <candidusl...@gmail.com>
    Signed-off-by: Matt Topol <zotthewiz...@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())
+               })
+       }
+}