Skip to content

Commit

Permalink
GH-35909: [Go] Deprecate arrow.MapType.ValueField & `arrow.MapType.…
Browse files Browse the repository at this point in the history
…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 <candiduslynx@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
  • Loading branch information
candiduslynx committed Jun 6, 2023
1 parent 77d8bc5 commit daacbcc
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 67 deletions.
6 changes: 3 additions & 3 deletions go/arrow/array/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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,
}
}
Expand Down
2 changes: 1 addition & 1 deletion go/arrow/cdata/cdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
24 changes: 16 additions & 8 deletions go/arrow/datatype_nested.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type (
ListLikeType interface {
DataType
Elem() DataType
ElemField() Field
}
)

Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions go/arrow/datatype_nested_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -477,19 +477,19 @@ 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)
}

if got, want := got.String(), tc.str; got != want {
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)
}
})
}
Expand Down
8 changes: 4 additions & 4 deletions go/arrow/internal/arrdata/arrdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
Expand All @@ -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]),
Expand All @@ -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]),
Expand All @@ -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]),
Expand Down
4 changes: 2 additions & 2 deletions go/arrow/internal/arrjson/arrjson.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go/arrow/ipc/file_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go/arrow/ipc/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 1 addition & 14 deletions go/arrow/scalar/nested.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion go/arrow/scalar/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 7 additions & 15 deletions go/parquet/pqarrow/encode_arrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 3 additions & 11 deletions go/parquet/pqarrow/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down

0 comments on commit daacbcc

Please sign in to comment.