hcrosse commented on code in PR #1490:
URL: https://github.com/apache/arrow-go/pull/1490#discussion_r4528697416
##########
arrow/compute/exec/span.go:
##########
@@ -327,6 +327,22 @@ func (a *ArraySpan) FillFromScalar(val scalar.Scalar) {
int64(len(dataBuffer)), 1)
}
a.Buffers[2].Buf = dataBuffer
+ case typeID == arrow.BINARY_VIEW || typeID == arrow.STRING_VIEW:
+ sc := val.(scalar.BinaryScalar)
+ a.Buffers[1].Buf = arrow.Uint64Traits.CastToBytes(a.Scratch[:])
+ a.Buffers[1].Owner = nil
+ a.Buffers[1].SelfAlloc = false
+ a.Buffers[2].Buf, a.Buffers[2].Owner = nil, nil
Review Comment:
Thanks. I changed `MakeData` to omit a `nil` data buffer for view arrays.
This also fixes the IPC writer and `cdata.ExportArrowArray`, which failed on
the same arrays. `TestArraySpan_FillFromScalarRoundTripsViews` now concatenates
with mixed arrays, and `TestArraySpan_MakeDataInlineOnlyViews` covers
`SetMembers`. I skip dense unions in the concatenation check because
`array.Concatenate` does not support them.
##########
arrow/compute/scalar_nested.go:
##########
@@ -94,8 +94,7 @@ type listElementFunction struct {
func listElementScalarResultSupported(typ arrow.DataType, nullResult bool)
bool {
switch typ.ID() {
- case arrow.BINARY_VIEW, arrow.STRING_VIEW, arrow.LIST_VIEW,
arrow.LARGE_LIST_VIEW,
- arrow.DECIMAL32, arrow.DECIMAL64:
+ case arrow.LIST_VIEW, arrow.LARGE_LIST_VIEW, arrow.DECIMAL32,
arrow.DECIMAL64:
Review Comment:
I restored `scalar_nested.go` to main.
##########
arrow/scalar/scalar.go:
##########
@@ -675,6 +678,14 @@ func GetScalar(arr arrow.Array, idx int) (Scalar, error) {
return NewStringScalar(arr.Value(idx)), nil
case *array.LargeString:
return NewLargeStringScalar(arr.Value(idx)), nil
+ case *array.StringView:
+ return NewStringViewScalar(arr.Value(idx)), nil
+ case *array.BinaryView:
+ // An inline value lives in the view header, so copy rather than
+ // slice a data buffer.
Review Comment:
I reworded the comment to say that the scalar shares neither the array's
header nor its data buffers.
##########
arrow/scalar/append.go:
##########
@@ -57,10 +58,11 @@ func appendPrimitive[T primitives, B builder[T]](bldr B,
scalars []Scalar) {
}
func appendBinary(bldr binaryBuilder, scalars []Scalar) {
+ _, view := bldr.Type().(arrow.BinaryViewDataType)
var dataSize int
for _, s := range scalars {
s := s.(BinaryScalar)
- if s.IsValid() {
+ if s.IsValid() && (!view || !arrow.IsViewInline(len(s.Data())))
{
Review Comment:
For view builders, I now reserve each value that needs a data buffer before
`UnsafeAppend`, as `Append` does. I couldn't call `Append` directly because it
takes `string` on string builders and `[]byte` on binary builders. Inline
values still reserve nothing, and other binary builders keep the single
reservation from main.
--
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]
