zeroshade commented on code in PR #33906:
URL: https://github.com/apache/arrow/pull/33906#discussion_r1090815353
##########
go/arrow/array/record_test.go:
##########
@@ -390,6 +390,28 @@ func TestRecordBuilder(t *testing.T) {
if got, want := rec.ColumnName(0), schema.Field(0).Name; got != want {
t.Fatalf("invalid column name: got=%q, want=%q", got, want)
}
+
+ {
+ dt := arrow.MapOf(arrow.BinaryTypes.String,
arrow.BinaryTypes.String)
+ dt.KeysSorted = true
+ dt.SetItemNullable(false)
+ schema := arrow.NewSchema([]arrow.Field{
+ {Name: "map", Type: dt, Nullable: false},
+ },
+ nil,
+ )
+
+ b := array.NewRecordBuilder(mem, schema)
+ defer b.Release()
+
+ mb := b.Field(0).(*array.MapBuilder)
+ mb.Append(true)
+
mb.KeyBuilder().(*array.StringBuilder).AppendValues([]string{"1", "2", "3"},
nil)
+
mb.ItemBuilder().(*array.StringBuilder).AppendValues([]string{"a", "b", "c"},
nil)
+
+ rec := b.NewRecord()
+ defer rec.Release()
Review Comment:
shouldn't this be compared against something to ensure it created the map
properly?
##########
go/arrow/array/map.go:
##########
@@ -249,6 +265,10 @@ func (b *MapBuilder) NewArray() arrow.Array {
// NewMapArray creates a new Map array from the memory buffers used by the
builder, and
// resets the builder so it can be used again to build a new Map array.
func (b *MapBuilder) NewMapArray() (a *Map) {
+ if !b.etype.ItemField().Nullable && b.ItemBuilder().NullN() > 0 {
+ panic("arrow/array: item not nullable")
+ }
Review Comment:
Add a test for this please
##########
go/arrow/array/map.go:
##########
@@ -126,26 +126,25 @@ type MapBuilder struct {
// building using keys in sorted order for each value. The KeysSorted value
will just be
// used when creating the DataType for the map.
//
-// Example
+// # Example
Review Comment:
Go documentation doesn't need the `#` here. A single line without a period
becomes a header like so:
https://pkg.go.dev/github.com/apache/arrow/go/[email protected]/arrow/array#hdr-Example
##########
go/arrow/datatype_nested.go:
##########
@@ -205,6 +205,8 @@ func (t *FixedSizeListType) String() string {
return fmt.Sprintf("fixed_size_list<%s: %s>[%d]", t.elem.Name,
t.elem.Type, t.n)
}
+func (t *FixedSizeListType) SetElemNullable(n bool) { t.elem.Nullable = n }
+
Review Comment:
Add a test for this please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
