zeroshade commented on code in PR #1170:
URL: https://github.com/apache/arrow-go/pull/1170#discussion_r3769127500
##########
arrow/array.go:
##########
@@ -110,6 +110,12 @@ type Array interface {
// ValueStr returns the value at index as a string.
ValueStr(i int) string
+ // ValueAsAny returns the native Go value at index i, or nil if the
slot is null.
+ // Unlike GetOneForMarshal, values are not converted for JSON encoding
+ // (for example int8 stays int8, timestamps stay arrow.Timestamp, and
+ // nested values are []any / map[string]any of native values).
+ ValueAsAny(i int) any
Review Comment:
Adding a method to the exported `arrow.Array` interface is
source-incompatible for downstream implementations. A type satisfying the
interface before this commit now fails with `missing method ValueAsAny`.
Unless this targets an explicitly breaking release, could this be exposed
through a standalone helper or narrower optional interface instead?
##########
arrow/array/union.go:
##########
@@ -333,6 +333,19 @@ func (a *SparseUnion) GetOneForMarshal(i int) interface{} {
return []interface{}{typeID, data.GetOneForMarshal(i)}
}
+func (a *SparseUnion) ValueAsAny(i int) any {
+ typeID := a.RawTypeCodes()[i]
+
+ childID := a.ChildID(i)
+ data := a.Field(childID)
+
+ if data.IsNull(i) {
Review Comment:
This union slot itself is valid and still has a meaningful type code even
when its selected child is null. Returning bare `nil` loses that discriminator.
Please preserve the union representation, for example `[]any{typeID, nil}`,
consistently with non-null union values.
##########
arrow/array/struct.go:
##########
@@ -280,6 +280,18 @@ func (a *Struct) GetOneForMarshal(i int) interface{} {
return tmp
}
+func (a *Struct) ValueAsAny(i int) any {
+ if a.IsNull(i) {
+ return nil
+ }
+ tmp := make(map[string]any)
+ fieldList := a.data.dtype.(*arrow.StructType).Fields()
+ for j, d := range a.fields {
+ tmp[fieldList[j].Name] = d.ValueAsAny(i)
Review Comment:
Arrow permits duplicate struct field names, but assigning by name into a map
silently overwrites earlier fields. For `dup:int32=11, dup:int64=22`, this
returns only `{"dup": 22}`.
Please use a representation that preserves field order and duplicates, or
explicitly reject ambiguous schemas.
##########
arrow/extensions/variant.go:
##########
@@ -612,6 +612,17 @@ func (v *VariantArray) GetOneForMarshal(i int) any {
return val.Value()
}
+func (v *VariantArray) ValueAsAny(i int) any {
+ if v.IsNull(i) {
+ return nil
+ }
+ val, err := v.Value(i)
+ if err != nil {
Review Comment:
A decode error on a non-null variant is converted to `nil`, making malformed
data indistinguishable from an actual null slot. The reproduced case has
`IsNull(i) == false` and invalid metadata, but `ValueAsAny(i) == nil`.
Please preserve an error signal rather than silently converting corruption
to null.
--
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]