zeroshade commented on code in PR #34986:
URL: https://github.com/apache/arrow/pull/34986#discussion_r1162977089
##########
go/arrow/array/binary.go:
##########
@@ -56,9 +57,17 @@ func (a *Binary) Value(i int) []byte {
return a.valueBytes[a.valueOffsets[idx]:a.valueOffsets[idx+1]]
}
-// ValueString returns the string at index i without performing additional
allocations.
-// The string is only valid for the lifetime of the Binary array.
+// ValueString returns the string at index i
func (a *Binary) ValueString(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+ return base64.StdEncoding.EncodeToString(a.Value(i))
Review Comment:
same comment as for LargeBinary, this seems backwards. `ValueString` should
be returning the `*(*string)(unsafe.Pointer(&b))` and `ValueStr` should return
the base64 encoded version
##########
go/arrow/array/binary.go:
##########
@@ -192,6 +201,12 @@ func (a *LargeBinary) Value(i int) []byte {
}
func (a *LargeBinary) ValueString(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+ return base64.StdEncoding.EncodeToString(a.Value(i))
Review Comment:
I think this is backwards. Shouldn't `ValueString` be implemented as `return
*(*string)(unsafe.Pointer(&b))` while `ValueStr` returns the base64 encoded
version?
##########
go/arrow/array/boolean.go:
##########
@@ -55,6 +55,14 @@ func (a *Boolean) Value(i int) bool {
return bitutil.BitIsSet(a.values, a.array.data.offset+i)
}
+func (a *Boolean) ValueStr(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ } else {
+ return fmt.Sprintf("%v", a.Value(i))
Review Comment:
`strconv.FormatBool` ?
##########
go/arrow/array/extension.go:
##########
@@ -187,7 +185,7 @@ func (e *ExtensionArrayBase) setData(data *Data) {
// ValueString returns the value at index i as a string.
Review Comment:
update the docstring comment to be `ValueStr`
##########
go/arrow/array/fixed_size_list.go:
##########
@@ -46,6 +46,13 @@ func NewFixedSizeListData(data arrow.ArrayData)
*FixedSizeList {
func (a *FixedSizeList) ListValues() arrow.Array { return a.values }
+func (a *FixedSizeList) ValueStr(i int) string {
+ if !a.IsValid(i) {
+ return NullValueStr
+ }
+ sub := a.newListValue(i)
Review Comment:
this should have `defer sub.Release()`. can you add a test that uses
`memory.CheckedAllocator` to construct this and then calls `ValueStr` to verify
we don't have a memory leak?
##########
go/arrow/array/list.go:
##########
@@ -52,6 +52,13 @@ func NewListData(data arrow.ArrayData) *List {
func (a *List) ListValues() arrow.Array { return a.values }
+func (a *List) ValueStr(i int) string {
+ if !a.IsValid(i) {
+ return NullValueStr
+ }
+ return fmt.Sprintf("%v", a.newListValue(i))
Review Comment:
same comment as for FixedSizeList. needs to defer call release on the return
from `newListValue` and please add a test that verifies no leaked memory
##########
go/arrow/array/numeric.gen.go.tmpl:
##########
@@ -82,6 +82,32 @@ func (a *{{.Name}}) setData(data *Data) {
}
}
+func (a *{{.Name}}) ValueStr(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+{{if or (eq .Name "Date32") (eq .Name "Date64") -}}
+ return a.values[i].ToTime().Format("2006-01-02")
Review Comment:
these types have a `FormattedString()` method that you can call instead.
just do `a.values[i].FormattedString()`
##########
go/arrow/array/numeric.gen.go.tmpl:
##########
@@ -82,6 +82,32 @@ func (a *{{.Name}}) setData(data *Data) {
}
}
+func (a *{{.Name}}) ValueStr(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+{{if or (eq .Name "Date32") (eq .Name "Date64") -}}
+ return a.values[i].ToTime().Format("2006-01-02")
+{{else if or (eq .Name "Time32") (eq .Name "Time64") -}}
+ return
a.values[i].ToTime(a.DataType().(*{{.QualifiedType}}Type).Unit).Format("15:04:05.999999999")
+{{else if or (eq .Name "Timestamp") -}}
+ return
a.values[i].ToTime(a.DataType().(*{{.QualifiedType}}Type).Unit).Format("2006-01-02
15:04:05.999999999")
+{{else if (eq .Name "Duration") -}}
+ // return value and suffix as a string such as "12345ms"
+ return fmt.Sprintf("%d%s", a.values[i],
a.DataType().(*{{.QualifiedType}}Type).Unit.String())
Review Comment:
you don't need the `.String()` since you're already using `%s` in the format
##########
go/arrow/array/numeric.gen.go.tmpl:
##########
@@ -82,6 +82,32 @@ func (a *{{.Name}}) setData(data *Data) {
}
}
+func (a *{{.Name}}) ValueStr(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+{{if or (eq .Name "Date32") (eq .Name "Date64") -}}
+ return a.values[i].ToTime().Format("2006-01-02")
+{{else if or (eq .Name "Time32") (eq .Name "Time64") -}}
+ return
a.values[i].ToTime(a.DataType().(*{{.QualifiedType}}Type).Unit).Format("15:04:05.999999999")
Review Comment:
`Time32` and `Time64` also have `FormattedString` methods which take the
unit. so this should be:
`return
a.values[i].FormattedString(a.DataType().(*{{.QualifiedType}}Type).Unit)`
##########
go/arrow/array/numericbuilder.gen.go.tmpl:
##########
@@ -184,6 +184,124 @@ func (b *{{.Name}}Builder) newData() (data *Data) {
return
}
+func (b *{{.Name}}Builder) AppendValueFromString(s string) error {
+ if s == NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ {{if or (eq .Name "Date32") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date32FromTime(tm))
+ {{else if or (eq .Name "Date64") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date64FromTime(tm))
+ {{else if or (eq .Name "Time32") -}}
+ val, err := arrow.Time32FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Time64") -}}
+ val, err := arrow.Time64FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Timestamp") -}}
+ v, err := arrow.TimestampFromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(v)
+ {{else if (eq .Name "Duration") -}}
+ return fmt.Errorf("AppendValueFromString not implemented for Duration")
Review Comment:
please prefix the error with `%w` and use `arrow.ErrNotImplemented` so that
we consistently wrap that error for this scenario.
##########
go/arrow/array/numericbuilder.gen.go.tmpl:
##########
@@ -184,6 +184,124 @@ func (b *{{.Name}}Builder) newData() (data *Data) {
return
}
+func (b *{{.Name}}Builder) AppendValueFromString(s string) error {
+ if s == NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ {{if or (eq .Name "Date32") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date32FromTime(tm))
+ {{else if or (eq .Name "Date64") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date64FromTime(tm))
+ {{else if or (eq .Name "Time32") -}}
+ val, err := arrow.Time32FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Time64") -}}
+ val, err := arrow.Time64FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Timestamp") -}}
+ v, err := arrow.TimestampFromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(v)
+ {{else if (eq .Name "Duration") -}}
+ return fmt.Errorf("AppendValueFromString not implemented for Duration")
+ {{else if or (eq .Name "Int8") -}}
+ v, err := strconv.ParseInt(s, 10, 8)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int8(v))
Review Comment:
You can condense all of these to:
```
v, err := strconv.ParseInt(s, 10, {{.Size}})
if err != nil {
b.AppendNull()
return err
}
b.Append({{.name}}(v))
```
instead of repeating it
##########
go/arrow/array/binary.go:
##########
@@ -105,7 +114,7 @@ func (a *Binary) String() string {
case a.IsNull(i):
o.WriteString("(null)")
default:
- fmt.Fprintf(o, "%q", a.ValueString(i))
+ fmt.Fprintf(o, "%q", a.ValueStr(i))
Review Comment:
this entire switch can probably be replaced with just calling
`a.ValueStr(i)` right? since it already does the `a.IsNull(i)` check
##########
go/arrow/array/binarybuilder_test.go:
##########
@@ -50,11 +51,16 @@ func TestBinaryBuilder(t *testing.T) {
}
assert.Equal(t, v, ab.Value(i), "unexpected
BinaryArrayBuilder.Value(%d)", i)
}
+ // Zm9v is foo in base64
+ assert.NoError(t, ab.AppendValueFromString("Zm9v"))
ar := ab.NewBinaryArray()
+ assert.Equal(t, "Zm9v", ar.ValueString(5))
+
ab.Release()
ar.Release()
+
Review Comment:
just running `go fmt` would clear up the extraneous extra lines
##########
go/arrow/array/float16.go:
##########
@@ -39,6 +39,7 @@ func NewFloat16Data(data arrow.ArrayData) *Float16 {
}
func (a *Float16) Value(i int) float16.Num { return a.values[i] }
+func (a *Float16) ValueStr(i int) string { return fmt.Sprintf("%v",
a.Value(i).Float32())}
Review Comment:
`float16.Num` has a `String()` method. You can just return
`a.values[i].String()`
##########
go/arrow/array/encoded.go:
##########
@@ -192,6 +193,15 @@ func (r *RunEndEncoded) GetPhysicalLength() int {
return encoded.GetPhysicalLength(r.data)
}
+func (r *RunEndEncoded) ValueStr(i int) string {
Review Comment:
Should this be taking a physical index or a logical index? I personally
think this should be a logical index and should use `FindPhysicalIndex` to
return `r.values.ValueStr(encoded.FindPhysicalIndex(r.Data(), i))` thoughts?
##########
go/arrow/array/string.go:
##########
@@ -188,6 +195,7 @@ func (a *LargeString) Value(i int) string {
i = i + a.array.data.offset
return a.values[a.offsets[i]:a.offsets[i+1]]
}
+func (a *LargeString) ValueStr(i int) string { return fmt.Sprintf("%q",
a.Value(i)) }
Review Comment:
why not just return `a.Value(i)` ?
##########
go/arrow/array/binarybuilder_test.go:
##########
@@ -38,6 +38,7 @@ func TestBinaryBuilder(t *testing.T) {
ab.AppendNull()
} else {
ab.Append(v)
+
Review Comment:
unnecessary empty line
##########
go/arrow/array/list.go:
##########
@@ -175,6 +182,12 @@ func NewLargeListData(data arrow.ArrayData) *LargeList {
func (a *LargeList) ListValues() arrow.Array { return a.values }
+func (a *LargeList) ValueStr(i int) string {
+ if !a.IsValid(i) {
+ return NullValueStr
+ }
+ return fmt.Sprintf("%v", a.newListValue(i))
Review Comment:
same comment as above
##########
go/arrow/array/struct.go:
##########
@@ -351,6 +360,14 @@ func (b *StructBuilder) newData() (data *Data) {
return
}
+func (b *StructBuilder) AppendValueFromString(s string) error {
+ if !strings.HasPrefix(s, "{") && !strings.HasSuffix(s, "}") {
+ return fmt.Errorf("invalid string for struct")
Review Comment:
add `%w` and wrap `arrow.ErrInvalid` please
##########
go/arrow/array/list.go:
##########
@@ -416,6 +429,9 @@ func (b *baseListBuilder) AppendEmptyValue() {
b.Append(true)
}
+// func (b *ListBuilder) AppendFromString(s string) {
+// b.AppendFromStringArray([]string{s})
+// }
Review Comment:
remove commented code?
##########
go/arrow/array/map.go:
##########
@@ -300,6 +300,10 @@ func (b *MapBuilder) ValueBuilder() Builder {
return b.listBuilder.ValueBuilder()
}
+func (b *MapBuilder) AppendValueFromString(s string) error {
+ panic("not implemented")
Review Comment:
can we just return `arrow.ErrNotImplemented` instead of panicing?
##########
go/arrow/array/numeric.gen.go.tmpl:
##########
@@ -82,6 +82,32 @@ func (a *{{.Name}}) setData(data *Data) {
}
}
+func (a *{{.Name}}) ValueStr(i int) string {
+ if a.IsNull(i) {
+ return NullValueStr
+ }
+{{if or (eq .Name "Date32") (eq .Name "Date64") -}}
+ return a.values[i].ToTime().Format("2006-01-02")
+{{else if or (eq .Name "Time32") (eq .Name "Time64") -}}
+ return
a.values[i].ToTime(a.DataType().(*{{.QualifiedType}}Type).Unit).Format("15:04:05.999999999")
+{{else if or (eq .Name "Timestamp") -}}
+ return
a.values[i].ToTime(a.DataType().(*{{.QualifiedType}}Type).Unit).Format("2006-01-02
15:04:05.999999999")
+{{else if (eq .Name "Duration") -}}
+ // return value and suffix as a string such as "12345ms"
+ return fmt.Sprintf("%d%s", a.values[i],
a.DataType().(*{{.QualifiedType}}Type).Unit.String())
+{{else if or (eq .Name "Int8") (eq .Name "Int16") (eq .Name "Int32") (eq .Name
"Int64") -}}
+ return strconv.FormatInt(int64(a.Value(i)), 10)
+{{else if or (eq .Name "Uint8") (eq .Name "Uint16") (eq .Name "Uint32") (eq
.Name "Uint64") -}}
+ return strconv.FormatUint(uint64(a.Value(i)), 10)
Review Comment:
any reason not to just use `fmt.Sprintf("%d", a.values[i])`
##########
go/arrow/array/numericbuilder.gen.go.tmpl:
##########
@@ -184,6 +184,124 @@ func (b *{{.Name}}Builder) newData() (data *Data) {
return
}
+func (b *{{.Name}}Builder) AppendValueFromString(s string) error {
+ if s == NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ {{if or (eq .Name "Date32") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date32FromTime(tm))
Review Comment:
fix the indentation formatting please
##########
go/arrow/array/numericbuilder.gen_test.go.tmpl:
##########
@@ -51,9 +51,10 @@ func TestNew{{.Name}}Builder(t *testing.T) {
ab.Append(8)
ab.Append(9)
ab.Append(10)
+ ab.AppendValueFromString(11)
Review Comment:
indentation
##########
go/arrow/array/struct.go:
##########
@@ -81,6 +81,15 @@ func NewStructData(data arrow.ArrayData) *Struct {
func (a *Struct) NumField() int { return len(a.fields) }
func (a *Struct) Field(i int) arrow.Array { return a.fields[i] }
+func (a *Struct) ValueStr(i int) string {
+ var buf bytes.Buffer
+ enc := json.NewEncoder(&buf)
+ if err := enc.Encode(a.GetOneForMarshal(i)); err != nil {
+ return ""
Review Comment:
we shouldn't swallow the error. either return `err.String()` or `panic(err)`
please
##########
go/arrow/array/numericbuilder.gen.go.tmpl:
##########
@@ -184,6 +184,124 @@ func (b *{{.Name}}Builder) newData() (data *Data) {
return
}
+func (b *{{.Name}}Builder) AppendValueFromString(s string) error {
+ if s == NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ {{if or (eq .Name "Date32") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date32FromTime(tm))
+ {{else if or (eq .Name "Date64") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date64FromTime(tm))
+ {{else if or (eq .Name "Time32") -}}
+ val, err := arrow.Time32FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Time64") -}}
+ val, err := arrow.Time64FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Timestamp") -}}
+ v, err := arrow.TimestampFromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(v)
+ {{else if (eq .Name "Duration") -}}
+ return fmt.Errorf("AppendValueFromString not implemented for Duration")
+ {{else if or (eq .Name "Int8") -}}
+ v, err := strconv.ParseInt(s, 10, 8)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int8(v))
+ {{else if or (eq .Name "Int16") -}}
+ v, err := strconv.ParseInt(s, 10, 16)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int16(v))
+ {{else if or (eq .Name "Int32") -}}
+ v, err := strconv.ParseInt(s, 10, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int32(v))
+ {{else if or (eq .Name "Int64") -}}
+ v, err := strconv.ParseInt(s, 10, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int64(v))
+ {{else if or (eq .Name "Uint8") -}}
+ v, err := strconv.ParseUint(s, 10, 8)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint8(v))
+ {{else if or (eq .Name "Uint16") -}}
+ v, err := strconv.ParseUint(s, 10, 16)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint16(v))
+ {{else if or (eq .Name "Uint32") -}}
+ v, err := strconv.ParseUint(s, 10, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint32(v))
+ {{else if or (eq .Name "Uint64") -}}
+ v, err := strconv.ParseUint(s, 10, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint64(v))
+ {{else if or (eq .Name "Float32") -}}
+ v, err := strconv.ParseFloat(s, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(float32(v))
+ {{else if or (eq .Name "Float64") -}}
+ v, err := strconv.ParseFloat(s, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(float64(v))
Review Comment:
same thing as with the integral types, you can condense these by using
`{{.Size}}` and `{{.name}}`
##########
go/arrow/array/numericbuilder.gen.go.tmpl:
##########
@@ -184,6 +184,124 @@ func (b *{{.Name}}Builder) newData() (data *Data) {
return
}
+func (b *{{.Name}}Builder) AppendValueFromString(s string) error {
+ if s == NullValueStr {
+ b.AppendNull()
+ return nil
+ }
+ {{if or (eq .Name "Date32") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date32FromTime(tm))
+ {{else if or (eq .Name "Date64") -}}
+ tm, err := time.Parse("2006-01-02", s)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(arrow.Date64FromTime(tm))
+ {{else if or (eq .Name "Time32") -}}
+ val, err := arrow.Time32FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Time64") -}}
+ val, err := arrow.Time64FromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(val)
+ {{else if or (eq .Name "Timestamp") -}}
+ v, err := arrow.TimestampFromString(s, b.dtype.Unit)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(v)
+ {{else if (eq .Name "Duration") -}}
+ return fmt.Errorf("AppendValueFromString not implemented for Duration")
+ {{else if or (eq .Name "Int8") -}}
+ v, err := strconv.ParseInt(s, 10, 8)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int8(v))
+ {{else if or (eq .Name "Int16") -}}
+ v, err := strconv.ParseInt(s, 10, 16)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int16(v))
+ {{else if or (eq .Name "Int32") -}}
+ v, err := strconv.ParseInt(s, 10, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int32(v))
+ {{else if or (eq .Name "Int64") -}}
+ v, err := strconv.ParseInt(s, 10, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(int64(v))
+ {{else if or (eq .Name "Uint8") -}}
+ v, err := strconv.ParseUint(s, 10, 8)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint8(v))
+ {{else if or (eq .Name "Uint16") -}}
+ v, err := strconv.ParseUint(s, 10, 16)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint16(v))
+ {{else if or (eq .Name "Uint32") -}}
+ v, err := strconv.ParseUint(s, 10, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint32(v))
+ {{else if or (eq .Name "Uint64") -}}
+ v, err := strconv.ParseUint(s, 10, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(uint64(v))
+ {{else if or (eq .Name "Float32") -}}
+ v, err := strconv.ParseFloat(s, 32)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(float32(v))
+ {{else if or (eq .Name "Float64") -}}
+ v, err := strconv.ParseFloat(s, 64)
+ if err != nil {
+ b.AppendNull()
+ return err
+ }
+ b.Append(float64(v))
+ {{else}}
+ return fmt.Errorf("AppendValueFromString not implemented for {{.Name}}")
Review Comment:
same comment as above, use `%w` and wrap the `arrow.ErrNotImplemented`
##########
go/arrow/array/struct.go:
##########
@@ -81,6 +81,15 @@ func NewStructData(data arrow.ArrayData) *Struct {
func (a *Struct) NumField() int { return len(a.fields) }
func (a *Struct) Field(i int) arrow.Array { return a.fields[i] }
+func (a *Struct) ValueStr(i int) string {
Review Comment:
add a docstring comment on here to indicate that the value is returned as a
JSON string please.
##########
go/arrow/array/union.go:
##########
@@ -343,6 +343,21 @@ func (a *SparseUnion) MarshalJSON() ([]byte, error) {
return buf.Bytes(), nil
}
+func (a *SparseUnion) Value(i int) string {
+ fieldList := a.unionType.Fields()
+ field := fieldList[a.ChildID(i)]
+ f := a.Field(a.ChildID(i))
+ return fmt.Sprintf("{%s=%v}", field.Name, f.GetOneForMarshal(i))
+}
Review Comment:
I think this is unnecesary. having the `ValueStr` is fine
--
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]
