You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ze...@apache.org on 2023/06/06 18:17:28 UTC

[arrow] branch main updated: GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods (#35899)

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 daacbcc4c5 GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods (#35899)
daacbcc4c5 is described below

commit daacbcc4c5f0e435b2158896584a4385dbf38986
Author: Alex Shcherbakov <ca...@users.noreply.github.com>
AuthorDate: Tue Jun 6 21:17:20 2023 +0300

    GH-35909: [Go] Deprecate `arrow.MapType.ValueField` & `arrow.MapType.ValueType` methods (#35899)
    
    ### Rationale for this change
    
    Follow-up for #35885
    
    ### What changes are included in this PR?
    
    * Added `ElemField() Field` to `arrow.ListLikeType` interface
    * Added `ElemField() Field` to `arrow.MapType` implementation
    * Added deprecation notice to `arrow.MapType.ValueField` & `arrow.MapType.ValueType`
    * Fixed a bug in `go/arrow/array/map.go` (`NewMapBuilderWithType` used `ValueType` instead of `ItemType`)
    
    ### Are these changes tested?
    
    Compile-time assertion for corresponding types.
    
    ### Are there any user-facing changes?
    
    * Added `ElemField() Field` to `arrow.ListLikeType` interface
    * Added `ElemField() Field` to `arrow.MapType` implementation
    * Added deprecation notice to `arrow.MapType.ValueField` & `arrow.MapType.ValueType`
    
    * Closes: #35909
    
    Authored-by: candiduslynx <ca...@gmail.com>
    Signed-off-by: Matt Topol <zo...@gmail.com>
---
 go/arrow/array/map.go                |  6 +++---
 go/arrow/cdata/cdata.go              |  2 +-
 go/arrow/datatype_nested.go          | 24 ++++++++++++++++--------
 go/arrow/datatype_nested_test.go     | 12 ++++++------
 go/arrow/internal/arrdata/arrdata.go |  8 ++++----
 go/arrow/internal/arrjson/arrjson.go |  4 ++--
 go/arrow/ipc/file_reader.go          |  2 +-
 go/arrow/ipc/metadata.go             |  2 +-
 go/arrow/scalar/nested.go            | 15 +--------------
 go/arrow/scalar/parse.go             |  2 +-
 go/parquet/pqarrow/encode_arrow.go   | 22 +++++++---------------
 go/parquet/pqarrow/schema.go         | 14 +++-----------
 12 files changed, 46 insertions(+), 67 deletions(-)

diff --git a/go/arrow/array/map.go b/go/arrow/array/map.go
index 815465c1e5..1e1ba59f0d 100644
--- a/go/arrow/array/map.go
+++ b/go/arrow/array/map.go
@@ -150,7 +150,7 @@ type MapBuilder struct {
 func NewMapBuilder(mem memory.Allocator, keytype, itemtype arrow.DataType, keysSorted bool) *MapBuilder {
 	etype := arrow.MapOf(keytype, itemtype)
 	etype.KeysSorted = keysSorted
-	listBldr := NewListBuilder(mem, etype.ValueType())
+	listBldr := NewListBuilder(mem, etype.Elem())
 	keyBldr := listBldr.ValueBuilder().(*StructBuilder).FieldBuilder(0)
 	keyBldr.Retain()
 	itemBldr := listBldr.ValueBuilder().(*StructBuilder).FieldBuilder(1)
@@ -167,7 +167,7 @@ func NewMapBuilder(mem memory.Allocator, keytype, itemtype arrow.DataType, keysS
 }
 
 func NewMapBuilderWithType(mem memory.Allocator, dt *arrow.MapType) *MapBuilder {
-	listBldr := NewListBuilder(mem, dt.ValueType())
+	listBldr := NewListBuilder(mem, dt.Elem())
 	keyBldr := listBldr.ValueBuilder().(*StructBuilder).FieldBuilder(0)
 	keyBldr.Retain()
 	itemBldr := listBldr.ValueBuilder().(*StructBuilder).FieldBuilder(1)
@@ -178,7 +178,7 @@ func NewMapBuilderWithType(mem memory.Allocator, dt *arrow.MapType) *MapBuilder
 		itemBuilder: itemBldr,
 		etype:       dt,
 		keytype:     dt.KeyType(),
-		itemtype:    dt.ValueType(),
+		itemtype:    dt.ItemType(),
 		keysSorted:  dt.KeysSorted,
 	}
 }
diff --git a/go/arrow/cdata/cdata.go b/go/arrow/cdata/cdata.go
index 638d39b02e..e20a12bbdf 100644
--- a/go/arrow/cdata/cdata.go
+++ b/go/arrow/cdata/cdata.go
@@ -359,7 +359,7 @@ func (imp *cimporter) doImportChildren() error {
 			imp.children[i].importChild(imp, c)
 		}
 	case arrow.MAP: // only one child to import, it's a struct array
-		imp.children[0].dt = imp.dt.(*arrow.MapType).ValueType()
+		imp.children[0].dt = imp.dt.(*arrow.MapType).Elem()
 		if err := imp.children[0].importChild(imp, children[0]); err != nil {
 			return err
 		}
diff --git a/go/arrow/datatype_nested.go b/go/arrow/datatype_nested.go
index 314172a21b..b2abfafdda 100644
--- a/go/arrow/datatype_nested.go
+++ b/go/arrow/datatype_nested.go
@@ -37,6 +37,7 @@ type (
 	ListLikeType interface {
 		DataType
 		Elem() DataType
+		ElemField() Field
 	}
 )
 
@@ -391,15 +392,22 @@ func (t *MapType) String() string {
 	return o.String()
 }
 
-func (t *MapType) KeyField() Field        { return t.value.Elem().(*StructType).Field(0) }
-func (t *MapType) KeyType() DataType      { return t.KeyField().Type }
-func (t *MapType) ItemField() Field       { return t.value.Elem().(*StructType).Field(1) }
-func (t *MapType) ItemType() DataType     { return t.ItemField().Type }
-func (t *MapType) ValueType() *StructType { return t.value.Elem().(*StructType) }
-func (t *MapType) ValueField() Field      { return Field{Name: "entries", Type: t.ValueType()} }
+func (t *MapType) KeyField() Field    { return t.value.Elem().(*StructType).Field(0) }
+func (t *MapType) KeyType() DataType  { return t.KeyField().Type }
+func (t *MapType) ItemField() Field   { return t.value.Elem().(*StructType).Field(1) }
+func (t *MapType) ItemType() DataType { return t.ItemField().Type }
+
+// Deprecated: use MapType.Elem().(*StructType) instead
+func (t *MapType) ValueType() *StructType { return t.Elem().(*StructType) }
+
+// Deprecated: use MapType.ElemField() instead
+func (t *MapType) ValueField() Field { return t.ElemField() }
 
 // Elem returns the MapType's element type (if treating MapType as ListLikeType)
-func (t *MapType) Elem() DataType { return t.ValueType() }
+func (t *MapType) Elem() DataType { return t.value.Elem() }
+
+// ElemField returns the MapType's element field (if treating MapType as ListLikeType)
+func (t *MapType) ElemField() Field { return Field{Name: "entries", Type: t.Elem()} }
 
 func (t *MapType) SetItemNullable(nullable bool) {
 	t.value.Elem().(*StructType).fields[1].Nullable = nullable
@@ -419,7 +427,7 @@ func (t *MapType) Fingerprint() string {
 	return fingerprint + "{" + keyFingerprint + itemFingerprint + "}"
 }
 
-func (t *MapType) Fields() []Field { return []Field{t.ValueField()} }
+func (t *MapType) Fields() []Field { return []Field{t.ElemField()} }
 
 func (t *MapType) Layout() DataTypeLayout {
 	return t.value.Layout()
diff --git a/go/arrow/datatype_nested_test.go b/go/arrow/datatype_nested_test.go
index 15d7b2e0e3..17d530ba85 100644
--- a/go/arrow/datatype_nested_test.go
+++ b/go/arrow/datatype_nested_test.go
@@ -417,7 +417,7 @@ func TestMapOf(t *testing.T) {
 				t.Fatalf("invalid item type. got=%q, want=%q", got, want)
 			}
 
-			if got, want := got.ValueType(), StructOf(got.KeyField(), got.ItemField()); !TypeEqual(got, want) {
+			if got, want := got.Elem(), StructOf(got.KeyField(), got.ItemField()); !TypeEqual(got, want) {
 				t.Fatalf("invalid value type. got=%q, want=%q", got, want)
 			}
 
@@ -477,7 +477,7 @@ func TestMapOfWithMetadata(t *testing.T) {
 				t.Fatalf("invalid item type. got=%q, want=%q", got, want)
 			}
 
-			if got, want := got.ValueType(), StructOf(got.KeyField(), got.ItemField()); !TypeEqual(got, want) {
+			if got, want := got.Elem(), StructOf(got.KeyField(), got.ItemField()); !TypeEqual(got, want) {
 				t.Fatalf("invalid value type. got=%q, want=%q", got, want)
 			}
 
@@ -485,11 +485,11 @@ func TestMapOfWithMetadata(t *testing.T) {
 				t.Fatalf("invalid String() result. got=%q, want=%q", got, want)
 			}
 
-			if !reflect.DeepEqual(got.ValueType().fields[0].Metadata, tc.keyMetadata) {
-				t.Fatalf("invalid key metadata. got=%v, want=%v", got.ValueType().fields[0].Metadata, tc.keyMetadata)
+			if !reflect.DeepEqual(got.Elem().(*StructType).fields[0].Metadata, tc.keyMetadata) {
+				t.Fatalf("invalid key metadata. got=%v, want=%v", got.Elem().(*StructType).fields[0].Metadata, tc.keyMetadata)
 			}
-			if !reflect.DeepEqual(got.ValueType().fields[1].Metadata, tc.itemMetadata) {
-				t.Fatalf("invalid item metadata. got=%v, want=%v", got.ValueType().fields[1].Metadata, tc.itemMetadata)
+			if !reflect.DeepEqual(got.Elem().(*StructType).fields[1].Metadata, tc.itemMetadata) {
+				t.Fatalf("invalid item metadata. got=%v, want=%v", got.Elem().(*StructType).fields[1].Metadata, tc.itemMetadata)
 			}
 		})
 	}
diff --git a/go/arrow/internal/arrdata/arrdata.go b/go/arrow/internal/arrdata/arrdata.go
index 3f023e40d6..311645fd3c 100644
--- a/go/arrow/internal/arrdata/arrdata.go
+++ b/go/arrow/internal/arrdata/arrdata.go
@@ -793,7 +793,7 @@ func makeMapsRecords() []arrow.Record {
 	chunks := [][]arrow.Array{
 		{
 			mapOf(mem, dtype.KeysSorted, []arrow.Array{
-				structOf(mem, dtype.ValueType(), [][]arrow.Array{
+				structOf(mem, dtype.Elem().(*arrow.StructType), [][]arrow.Array{
 					{
 						arrayOf(mem, []int32{-1, -2, -3, -4, -5}, nil),
 						arrayOf(mem, []string{"111", "222", "333", "444", "555"}, mask[:5]),
@@ -815,7 +815,7 @@ func makeMapsRecords() []arrow.Record {
 						arrayOf(mem, []string{"4111", "4222", "4333", "4444", "4555"}, mask[:5]),
 					},
 				}, nil),
-				structOf(mem, dtype.ValueType(), [][]arrow.Array{
+				structOf(mem, dtype.Elem().(*arrow.StructType), [][]arrow.Array{
 					{
 						arrayOf(mem, []int32{1, 2, 3, 4, 5}, nil),
 						arrayOf(mem, []string{"-111", "-222", "-333", "-444", "-555"}, mask[:5]),
@@ -841,7 +841,7 @@ func makeMapsRecords() []arrow.Record {
 		},
 		{
 			mapOf(mem, dtype.KeysSorted, []arrow.Array{
-				structOf(mem, dtype.ValueType(), [][]arrow.Array{
+				structOf(mem, dtype.Elem().(*arrow.StructType), [][]arrow.Array{
 					{
 						arrayOf(mem, []int32{1, 2, 3, 4, 5}, nil),
 						arrayOf(mem, []string{"-111", "-222", "-333", "-444", "-555"}, mask[:5]),
@@ -863,7 +863,7 @@ func makeMapsRecords() []arrow.Record {
 						arrayOf(mem, []string{"-4111", "-4222", "-4333", "-4444", "-4555"}, mask[:5]),
 					},
 				}, nil),
-				structOf(mem, dtype.ValueType(), [][]arrow.Array{
+				structOf(mem, dtype.Elem().(*arrow.StructType), [][]arrow.Array{
 					{
 						arrayOf(mem, []int32{-1, -2, -3, -4, -5}, nil),
 						arrayOf(mem, []string{"111", "222", "333", "444", "555"}, mask[:5]),
diff --git a/go/arrow/internal/arrjson/arrjson.go b/go/arrow/internal/arrjson/arrjson.go
index 93e3039e98..f25691f0cf 100644
--- a/go/arrow/internal/arrjson/arrjson.go
+++ b/go/arrow/internal/arrjson/arrjson.go
@@ -1098,7 +1098,7 @@ func arrayFromJSON(mem memory.Allocator, dt arrow.DataType, arr Array) arrow.Arr
 
 	case *arrow.MapType:
 		valids := validsFromJSON(arr.Valids)
-		elems := arrayFromJSON(mem, dt.ValueType(), arr.Children[0])
+		elems := arrayFromJSON(mem, dt.Elem(), arr.Children[0])
 		defer elems.Release()
 
 		bitmap := validsToBitmap(valids, mem)
@@ -1429,7 +1429,7 @@ func arrayToJSON(field arrow.Field, arr arrow.Array) Array {
 			Valids: validsToJSON(arr),
 			Offset: arr.Offsets(),
 			Children: []Array{
-				arrayToJSON(arrow.Field{Name: "entries", Type: arr.DataType().(*arrow.MapType).ValueType()}, arr.ListValues()),
+				arrayToJSON(arrow.Field{Name: "entries", Type: arr.DataType().(*arrow.MapType).Elem()}, arr.ListValues()),
 			},
 		}
 		return o
diff --git a/go/arrow/ipc/file_reader.go b/go/arrow/ipc/file_reader.go
index a561352960..d12f57324d 100644
--- a/go/arrow/ipc/file_reader.go
+++ b/go/arrow/ipc/file_reader.go
@@ -589,7 +589,7 @@ func (ctx *arrayLoaderContext) loadMap(dt *arrow.MapType) arrow.ArrayData {
 	buffers = append(buffers, ctx.buffer())
 	defer releaseBuffers(buffers)
 
-	sub := ctx.loadChild(dt.ValueType())
+	sub := ctx.loadChild(dt.Elem())
 	defer sub.Release()
 
 	return array.NewData(dt, int(field.Length()), buffers, []arrow.ArrayData{sub}, int(field.NullCount()), 0)
diff --git a/go/arrow/ipc/metadata.go b/go/arrow/ipc/metadata.go
index a489c94ceb..eaa04beeb0 100644
--- a/go/arrow/ipc/metadata.go
+++ b/go/arrow/ipc/metadata.go
@@ -420,7 +420,7 @@ func (fv *fieldVisitor) visit(field arrow.Field) {
 
 	case *arrow.MapType:
 		fv.dtype = flatbuf.TypeMap
-		fv.kids = append(fv.kids, fieldToFB(fv.b, fv.pos.Child(0), dt.ValueField(), fv.memo))
+		fv.kids = append(fv.kids, fieldToFB(fv.b, fv.pos.Child(0), dt.ElemField(), fv.memo))
 		flatbuf.MapStart(fv.b)
 		flatbuf.MapAddKeysSorted(fv.b, dt.KeysSorted)
 		fv.offset = flatbuf.MapEnd(fv.b)
diff --git a/go/arrow/scalar/nested.go b/go/arrow/scalar/nested.go
index d777727e11..38fe3298e5 100644
--- a/go/arrow/scalar/nested.go
+++ b/go/arrow/scalar/nested.go
@@ -69,20 +69,7 @@ func (l *List) Validate() (err error) {
 		return
 	}
 
-	var (
-		valueType arrow.DataType
-	)
-
-	switch dt := l.Type.(type) {
-	case *arrow.ListType:
-		valueType = dt.Elem()
-	case *arrow.LargeListType:
-		valueType = dt.Elem()
-	case *arrow.FixedSizeListType:
-		valueType = dt.Elem()
-	case *arrow.MapType:
-		valueType = dt.ValueType()
-	}
+	valueType := l.Type.(arrow.ListLikeType).Elem()
 	listType := l.Type
 
 	if !arrow.TypeEqual(l.Value.DataType(), valueType) {
diff --git a/go/arrow/scalar/parse.go b/go/arrow/scalar/parse.go
index 1d50c5f9b9..2051c54b39 100644
--- a/go/arrow/scalar/parse.go
+++ b/go/arrow/scalar/parse.go
@@ -512,7 +512,7 @@ func MakeScalarParam(val interface{}, dt arrow.DataType) (Scalar, error) {
 			}
 			return NewFixedSizeListScalarWithType(v, dt), nil
 		case arrow.MAP:
-			if !arrow.TypeEqual(dt.(*arrow.MapType).ValueType(), v.DataType()) {
+			if !arrow.TypeEqual(dt.(*arrow.MapType).Elem(), v.DataType()) {
 				return nil, fmt.Errorf("inconsistent type for map scalar type")
 			}
 			return NewMapScalar(v), nil
diff --git a/go/parquet/pqarrow/encode_arrow.go b/go/parquet/pqarrow/encode_arrow.go
index dea412c8b9..a91cee3a73 100644
--- a/go/parquet/pqarrow/encode_arrow.go
+++ b/go/parquet/pqarrow/encode_arrow.go
@@ -36,25 +36,17 @@ import (
 
 // get the count of the number of leaf arrays for the type
 func calcLeafCount(dt arrow.DataType) int {
-	switch dt.ID() {
-	case arrow.EXTENSION:
-		return calcLeafCount(dt.(arrow.ExtensionType).StorageType())
-	case arrow.SPARSE_UNION, arrow.DENSE_UNION:
-		panic("arrow type not implemented")
-	case arrow.DICTIONARY:
-		return calcLeafCount(dt.(*arrow.DictionaryType).ValueType)
-	case arrow.LIST:
-		return calcLeafCount(dt.(*arrow.ListType).Elem())
-	case arrow.FIXED_SIZE_LIST:
-		return calcLeafCount(dt.(*arrow.FixedSizeListType).Elem())
-	case arrow.MAP:
-		return calcLeafCount(dt.(*arrow.MapType).ValueType())
-	case arrow.STRUCT:
+	switch dt := dt.(type) {
+	case arrow.ExtensionType:
+		return calcLeafCount(dt.StorageType())
+	case arrow.NestedType:
 		nleaves := 0
-		for _, f := range dt.(*arrow.StructType).Fields() {
+		for _, f := range dt.Fields() {
 			nleaves += calcLeafCount(f.Type)
 		}
 		return nleaves
+	case *arrow.DictionaryType:
+		return calcLeafCount(dt.ValueType)
 	default:
 		return 1
 	}
diff --git a/go/parquet/pqarrow/schema.go b/go/parquet/pqarrow/schema.go
index 73eece19c7..2e83d3926a 100644
--- a/go/parquet/pqarrow/schema.go
+++ b/go/parquet/pqarrow/schema.go
@@ -1002,7 +1002,7 @@ func applyOriginalStorageMetadata(origin arrow.Field, inferred *SchemaField) (mo
 			}
 			inferred.Field.Type = factory(modifiedChildren)
 		}
-	case arrow.FIXED_SIZE_LIST, arrow.LIST, arrow.MAP:
+	case arrow.FIXED_SIZE_LIST, arrow.LIST, arrow.LARGE_LIST, arrow.MAP: // arrow.ListLike
 		if nchildren != 1 {
 			return
 		}
@@ -1012,17 +1012,9 @@ func applyOriginalStorageMetadata(origin arrow.Field, inferred *SchemaField) (mo
 		}
 
 		modified = origin.Type.ID() != inferred.Field.Type.ID()
-		var childModified bool
-		switch typ := origin.Type.(type) {
-		case *arrow.FixedSizeListType:
-			childModified, err = applyOriginalMetadata(arrow.Field{Type: typ.Elem()}, &inferred.Children[0])
-		case *arrow.ListType:
-			childModified, err = applyOriginalMetadata(arrow.Field{Type: typ.Elem()}, &inferred.Children[0])
-		case *arrow.MapType:
-			childModified, err = applyOriginalMetadata(arrow.Field{Type: typ.ValueType()}, &inferred.Children[0])
-		}
+		childModified, err := applyOriginalMetadata(arrow.Field{Type: origin.Type.(arrow.ListLikeType).Elem()}, &inferred.Children[0])
 		if err != nil {
-			return
+			return modified, err
 		}
 		modified = modified || childModified
 		if modified {