zeroshade commented on code in PR #1138: URL: https://github.com/apache/arrow-go/pull/1138#discussion_r3884219864
########## arrow/compute/internal/kernels/scalar_nested.go: ########## @@ -0,0 +1,567 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.18 + +package kernels + +import ( + "fmt" + "math" + + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/bitutil" + "github.com/apache/arrow-go/v18/arrow/compute/exec" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/arrow/scalar" +) + +func listElementOutputType(_ *exec.KernelCtx, inputTypes []arrow.DataType) (arrow.DataType, error) { + listType, ok := inputTypes[0].(arrow.ListLikeType) + if !ok { + return nil, fmt.Errorf("%w: list_element requires a list-like input", arrow.ErrType) + } + return listType.Elem(), nil +} + +func getListElementIndex(value *exec.ExecValue) (uint64, error) { + if value.IsScalar() { + return listElementScalarIndex(value.Scalar) + } + + if value.Array.Len == 0 { + return 0, fmt.Errorf("%w: list_element index array is empty", arrow.ErrInvalid) + } + if value.Array.Len > 1 { + return 0, fmt.Errorf("%w: list_element does not support arrays of list indices", arrow.ErrNotImplemented) + } + if value.Array.UpdateNullCount() != 0 { + return 0, fmt.Errorf("%w: list_element index must not contain nulls", arrow.ErrInvalid) + } + + switch value.Array.Type.ID() { + case arrow.INT8: + return unsignedIndex(exec.GetSpanValues[int8](&value.Array, 1)[0]) + case arrow.INT16: + return unsignedIndex(exec.GetSpanValues[int16](&value.Array, 1)[0]) + case arrow.INT32: + return unsignedIndex(exec.GetSpanValues[int32](&value.Array, 1)[0]) + case arrow.INT64: + return unsignedIndex(exec.GetSpanValues[int64](&value.Array, 1)[0]) + case arrow.UINT8: + return uint64(exec.GetSpanValues[uint8](&value.Array, 1)[0]), nil + case arrow.UINT16: + return uint64(exec.GetSpanValues[uint16](&value.Array, 1)[0]), nil + case arrow.UINT32: + return uint64(exec.GetSpanValues[uint32](&value.Array, 1)[0]), nil + case arrow.UINT64: + return exec.GetSpanValues[uint64](&value.Array, 1)[0], nil + default: + return 0, fmt.Errorf("%w: invalid list_element index type %s", arrow.ErrType, value.Array.Type) + } +} + +func ValidateListElementScalarIndex(value scalar.Scalar) error { + _, err := listElementScalarIndex(value) + return err +} + +func ListElementScalarIndex(value scalar.Scalar) (uint64, error) { + return listElementScalarIndex(value) +} + +func listElementScalarIndex(value scalar.Scalar) (uint64, error) { + if !value.IsValid() { + return 0, fmt.Errorf("%w: list_element index must not be null", arrow.ErrInvalid) + } + return scalarIndex(value) +} + +func scalarIndex(value scalar.Scalar) (uint64, error) { + switch value := value.(type) { + case *scalar.Int8: + return unsignedIndex(value.Value) + case *scalar.Int16: + return unsignedIndex(value.Value) + case *scalar.Int32: + return unsignedIndex(value.Value) + case *scalar.Int64: + return unsignedIndex(value.Value) + case *scalar.Uint8: + return uint64(value.Value), nil + case *scalar.Uint16: + return uint64(value.Value), nil + case *scalar.Uint32: + return uint64(value.Value), nil + case *scalar.Uint64: + return value.Value, nil + default: + return 0, fmt.Errorf("%w: invalid list_element index type %s", arrow.ErrType, value.DataType()) + } +} + +func unsignedIndex[T arrow.IntType](value T) (uint64, error) { + if value < 0 { + return 0, fmt.Errorf("%w: list_element index %d is out of bounds: should be greater than or equal to 0", arrow.ErrInvalid, value) + } + return uint64(value), nil +} + +func listElementValueOffsets(list *exec.ArraySpan, i int64) (int64, int64, error) { + check := func(start, end int64) (int64, int64, error) { + if start < 0 || end < start || end > list.Children[0].Len { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + return start, end, nil + } + + switch list.Type.ID() { + case arrow.LIST: + offsets := exec.GetSpanOffsets[int32](list, 1) + return check(int64(offsets[i]), int64(offsets[i+1])) + case arrow.LARGE_LIST: + offsets := exec.GetSpanOffsets[int64](list, 1) + return check(offsets[i], offsets[i+1]) + case arrow.LIST_VIEW: + offsets := exec.GetSpanValues[int32](list, 1) + sizes := exec.GetSpanValues[int32](list, 2) + start := int64(offsets[i]) + size := int64(sizes[i]) + if size < 0 { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + return check(start, start+size) + case arrow.LARGE_LIST_VIEW: + offsets := exec.GetSpanValues[int64](list, 1) + sizes := exec.GetSpanValues[int64](list, 2) + start := offsets[i] + size := sizes[i] + if start < 0 || size < 0 || size > math.MaxInt64-start { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + return check(start, start+size) + case arrow.FIXED_SIZE_LIST: + size := int64(list.Type.(*arrow.FixedSizeListType).Len()) + if list.Offset < 0 || i < 0 || i > math.MaxInt64-list.Offset { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + position := list.Offset + i + if size < 0 || (size > 0 && position > math.MaxInt64/size) { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + start := position * size + if size > math.MaxInt64-start { + return 0, 0, fmt.Errorf("%w: list_element input has invalid value offsets", arrow.ErrInvalid) + } + return check(start, start+size) + default: + return 0, 0, fmt.Errorf("%w: unsupported list_element input type %s", arrow.ErrType, list.Type) + } +} + +func listElementExec(ctx *exec.KernelCtx, batch *exec.ExecSpan, out *exec.ExecResult) error { Review Comment: `list<string_view>` and `list<binary_view>` crash the process here, and the caller cannot recover from it: ``` child string_view buffers = 3 calling compute.ListElement inside recover()... returned datum kind=chunked_array <- the call SUCCEEDS SURVIVED - caller recover() was sufficient panic: runtime error: index out of range [3] with length 3 goroutine 8: exec.(*ArraySpan).TakeOwnership span.go:452 kernels.listElementConcat kernels/scalar_nested.go:296 compute.(*scalarExecutor).executeSpans executor.go:634 ``` `compute.ListElement` returns normally, and then an executor goroutine panics asynchronously — `recover()` in the calling goroutine is useless. It's reachable purely through the public API with a plain `list<string_view>`, and triggers once the child has 2 or more variadic data buffers. The root cause is that `ArraySpan.Buffers` is `[3]BufferSpan`, while a string/binary-view array carries validity + view-headers + N variadic data buffers, so `TakeOwnership` writes past index 2. Note that trimming trailing nil buffers in `ArraySpan.MakeData` does **not** fix this — I tried that first, and it only fixes the inlined case before hitting the out-of-bounds write. Please reject view element types recursively here, matching what the scalar path already does via `listElementScalarResultSupported`. The recursion is load-bearing: `struct<string_view>`, `list<string_view>`, `fsl<string_view,2>` and dictionary/REE children all need it, and a flat check leaves those crashing. With that in place both paths agree and return a consistent `ErrNotImplemented`. The underlying `ArraySpan` 3-buffer limitation should be its own issue — not this PR's to fix. ########## arrow/compute/scalar_nested_test.go: ########## @@ -0,0 +1,1992 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.18 + +package compute_test + +import ( + "context" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/compute" + "github.com/apache/arrow-go/v18/arrow/decimal" + "github.com/apache/arrow-go/v18/arrow/memory" + "github.com/apache/arrow-go/v18/arrow/scalar" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type denseUnionExtensionArray struct { + array.ExtensionArrayBase +} + +func (a denseUnionExtensionArray) ValueStr(i int) string { + if a.IsNull(i) { + return array.NullValueStr + } + return "dense_union" +} + +type denseUnionExtensionType struct { + arrow.ExtensionBase +} + +func (denseUnionExtensionType) ArrayType() reflect.Type { + return reflect.TypeOf(denseUnionExtensionArray{}) +} + +func (denseUnionExtensionType) ExtensionName() string { + return "compute-test.dense-union" +} + +func (t *denseUnionExtensionType) ExtensionEquals(other arrow.ExtensionType) bool { + rhs, ok := other.(*denseUnionExtensionType) + return ok && arrow.TypeEqual(t.StorageType(), rhs.StorageType()) +} + +func (denseUnionExtensionType) Serialize() string { return "" } + +func (t *denseUnionExtensionType) Deserialize(storage arrow.DataType, _ string) (arrow.ExtensionType, error) { + return &denseUnionExtensionType{ExtensionBase: arrow.ExtensionBase{Storage: storage}}, nil +} + +type runEndExtensionArray struct { + array.ExtensionArrayBase +} + +func (a runEndExtensionArray) ValueStr(i int) string { + return a.Storage().ValueStr(i) +} + +type runEndExtensionType struct { + arrow.ExtensionBase +} + +func (runEndExtensionType) ArrayType() reflect.Type { + return reflect.TypeOf(runEndExtensionArray{}) +} + +func (runEndExtensionType) ExtensionName() string { + return "compute-test.run-end" +} + +func (t *runEndExtensionType) ExtensionEquals(other arrow.ExtensionType) bool { + rhs, ok := other.(*runEndExtensionType) + return ok && arrow.TypeEqual(t.StorageType(), rhs.StorageType()) +} + +func (runEndExtensionType) Serialize() string { return "" } + +func (t *runEndExtensionType) Deserialize(storage arrow.DataType, _ string) (arrow.ExtensionType, error) { + return &runEndExtensionType{ExtensionBase: arrow.ExtensionBase{Storage: storage}}, nil +} + +func listElementInput(t *testing.T, mem memory.Allocator, typ arrow.DataType, values string) arrow.Array { + arr, _, err := array.FromJSON(mem, typ, strings.NewReader(values)) + require.NoError(t, err) + return arr +} + +func TestListElement(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + types := []arrow.DataType{ + arrow.ListOf(arrow.PrimitiveTypes.Int32), + arrow.LargeListOf(arrow.PrimitiveTypes.Int32), + arrow.ListViewOf(arrow.PrimitiveTypes.Int32), + arrow.LargeListViewOf(arrow.PrimitiveTypes.Int32), + arrow.FixedSizeListOf(2, arrow.PrimitiveTypes.Int32), + } + for _, typ := range types { + t.Run(typ.String(), func(t *testing.T) { + input := listElementInput(t, mem, typ, `[[1, 2], [3, 4], null, [5, 6]]`) + defer input.Release() + expected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[2, 4, null, 6]`) + defer expected.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: input.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(1)}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) + }) + } +} + +func TestListElementAllIntegerIndexTypes(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + listTypes := []arrow.DataType{ + arrow.ListOf(arrow.PrimitiveTypes.Int32), + arrow.LargeListOf(arrow.PrimitiveTypes.Int32), + arrow.ListViewOf(arrow.PrimitiveTypes.Int32), + arrow.LargeListViewOf(arrow.PrimitiveTypes.Int32), + arrow.FixedSizeListOf(2, arrow.PrimitiveTypes.Int32), + } + indexes := []struct { + name string + typ arrow.DataType + scalar scalar.Scalar + }{ + {name: "int8", typ: arrow.PrimitiveTypes.Int8, scalar: scalar.NewInt8Scalar(1)}, + {name: "int16", typ: arrow.PrimitiveTypes.Int16, scalar: scalar.NewInt16Scalar(1)}, + {name: "int32", typ: arrow.PrimitiveTypes.Int32, scalar: scalar.NewInt32Scalar(1)}, + {name: "int64", typ: arrow.PrimitiveTypes.Int64, scalar: scalar.NewInt64Scalar(1)}, + {name: "uint8", typ: arrow.PrimitiveTypes.Uint8, scalar: scalar.NewUint8Scalar(1)}, + {name: "uint16", typ: arrow.PrimitiveTypes.Uint16, scalar: scalar.NewUint16Scalar(1)}, + {name: "uint32", typ: arrow.PrimitiveTypes.Uint32, scalar: scalar.NewUint32Scalar(1)}, + {name: "uint64", typ: arrow.PrimitiveTypes.Uint64, scalar: scalar.NewUint64Scalar(1)}, + } + + for _, listType := range listTypes { + t.Run(listType.String(), func(t *testing.T) { + input := listElementInput(t, mem, listType, `[[10, 20], null]`) + defer input.Release() + expected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[20, null]`) + defer expected.Release() + + for _, tc := range indexes { + tc := tc + t.Run(tc.name+" scalar", func(t *testing.T) { + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: input.Data()}, + &compute.ScalarDatum{Value: tc.scalar}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + require.NoError(t, array.ValidateFull(actual)) + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) + }) + + t.Run(tc.name+" array", func(t *testing.T) { + singleInput := listElementInput(t, mem, listType, `[[10, 20]]`) + defer singleInput.Release() + index := listElementInput(t, mem, tc.typ, `[1]`) + defer index.Release() + expectedSingle := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[20]`) + defer expectedSingle.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: singleInput.Data()}, + &compute.ArrayDatum{Value: index.Data()}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + require.NoError(t, array.ValidateFull(actual)) + assert.True(t, array.Equal(expectedSingle, actual), "expected: %s\ngot: %s", expectedSingle, actual) + }) + } + }) + } +} + +func TestListElementNumericChildren(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + childTypes := []arrow.DataType{ + arrow.PrimitiveTypes.Int8, + arrow.PrimitiveTypes.Int16, + arrow.PrimitiveTypes.Int32, + arrow.PrimitiveTypes.Int64, + arrow.PrimitiveTypes.Uint8, + arrow.PrimitiveTypes.Uint16, + arrow.PrimitiveTypes.Uint32, + arrow.PrimitiveTypes.Uint64, + arrow.FixedWidthTypes.Float16, + arrow.PrimitiveTypes.Float32, + arrow.PrimitiveTypes.Float64, + } + for _, childType := range childTypes { + t.Run(childType.String(), func(t *testing.T) { + listTypes := []arrow.DataType{ + arrow.ListOf(childType), + arrow.LargeListOf(childType), + arrow.ListViewOf(childType), + arrow.LargeListViewOf(childType), + arrow.FixedSizeListOf(2, childType), + } + for _, listType := range listTypes { + t.Run(listType.String(), func(t *testing.T) { + input := listElementInput(t, mem, listType, `[[1, 2], [3, 4], null, [5, 6]]`) + defer input.Release() + expected := listElementInput(t, mem, childType, `[2, 4, null, 6]`) + defer expected.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: input.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(1)}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + require.NoError(t, array.ValidateFull(actual)) + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) + }) + } + }) + } +} + +func TestListElementPreservesChildNulls(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + input := listElementInput(t, mem, arrow.ListOf(arrow.PrimitiveTypes.Int32), `[[1, null], [2, 3]]`) + defer input.Release() + expected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[null, 3]`) + defer expected.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: input.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt8Scalar(1)}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) +} + +func TestListElementSlicedInputs(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + tests := []arrow.DataType{ + arrow.ListOf(arrow.PrimitiveTypes.Int32), + arrow.FixedSizeListOf(2, arrow.PrimitiveTypes.Int32), + } + for _, typ := range tests { + t.Run(typ.String(), func(t *testing.T) { + input := listElementInput(t, mem, typ, `[[0, 1], [2, 3], [4, 5], [6, 7]]`) + defer input.Release() + sliced := array.NewSlice(input, 1, 3) + defer sliced.Release() + expected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[3, 5]`) + defer expected.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: sliced.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(1)}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) + + }) + } +} + +func TestListElementListViewUsesSizes(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + tests := []struct { + name string + input arrow.Array + }{ + {name: "list view", input: makeListViewWithOutOfOrderOffsets(mem)}, + {name: "large list view", input: makeLargeListViewWithOutOfOrderOffsets(mem)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + defer tc.input.Release() + expected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[14, 10, 12]`) + defer expected.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: tc.input.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(0)}, + ) + require.NoError(t, err) + defer result.Release() + + actual := result.(*compute.ArrayDatum).MakeArray() + defer actual.Release() + assert.True(t, array.Equal(expected, actual), "expected: %s\ngot: %s", expected, actual) + + sliced := array.NewSlice(tc.input, 1, int64(tc.input.Len())) + defer sliced.Release() + slicedExpected := listElementInput(t, mem, arrow.PrimitiveTypes.Int32, `[10, 12]`) + defer slicedExpected.Release() + slicedResult, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: sliced.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(0)}, + ) + require.NoError(t, err) + defer slicedResult.Release() + slicedActual := slicedResult.(*compute.ArrayDatum).MakeArray() + defer slicedActual.Release() + assert.True(t, array.Equal(slicedExpected, slicedActual), "expected: %s\ngot: %s", slicedExpected, slicedActual) + }) + } +} + +func TestListElementRejectsInvalidListViewOffsets(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + + tests := []struct { + name string + build func(memory.Allocator) arrow.Array + }{ + {"list view negative offset", func(mem memory.Allocator) arrow.Array { + builder := array.NewListViewBuilder(mem, arrow.PrimitiveTypes.Int32) + builder.ValueBuilder().(*array.Int32Builder).Append(7) + builder.AppendDimensions(-1, 1) + result := builder.NewArray() + builder.Release() + return result + }}, + {"list view child overflow", func(mem memory.Allocator) arrow.Array { + builder := array.NewListViewBuilder(mem, arrow.PrimitiveTypes.Int32) + builder.ValueBuilder().(*array.Int32Builder).Append(7) + builder.AppendDimensions(1, 1) + result := builder.NewArray() + builder.Release() + return result + }}, + {"large list view negative offset", func(mem memory.Allocator) arrow.Array { + builder := array.NewLargeListViewBuilder(mem, arrow.PrimitiveTypes.Int32) + builder.ValueBuilder().(*array.Int32Builder).Append(7) + builder.AppendDimensions(-1, 1) + result := builder.NewArray() + builder.Release() + return result + }}, + {"large list view child overflow", func(mem memory.Allocator) arrow.Array { + builder := array.NewLargeListViewBuilder(mem, arrow.PrimitiveTypes.Int32) + builder.ValueBuilder().(*array.Int32Builder).Append(7) + builder.AppendDimensions(1, 1) + result := builder.NewArray() + builder.Release() + return result + }}, + {"large list view offset overflow", func(mem memory.Allocator) arrow.Array { + builder := array.NewLargeListViewBuilder(mem, arrow.PrimitiveTypes.Int32) + builder.AppendDimensions(int(^uint(0)>>1), 1) + result := builder.NewArray() + builder.Release() + return result + }}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + input := tc.build(mem) + defer input.Release() + + result, err := compute.ListElement( + context.Background(), + &compute.ArrayDatum{Value: input.Data()}, + &compute.ScalarDatum{Value: scalar.NewInt64Scalar(0)}, + ) + if result != nil { Review Comment: Releasing on the error path makes the compute suite flaky under `-race`. `compute.ListElement` intermittently returns a non-nil `Datum` wrapping a nil `*arrow.Chunked`, so `Release()` segfaults in `arrow.(*Chunked).Release(0x0)`. Full `go test -race ./arrow/compute/... ./arrow/array/` failed 1 of 3 runs; a targeted loop reproduced it 5/5. This pattern appears at 13 sites in this file — lines 430, 460, 656, 760, 807, 862, 925, 937, 996, 1311, 1362, 1424, 1867 and 1904. Line 1867 sits inside a 20-iteration loop, which multiplies the odds considerably. Guarding all of them with `if err == nil && result != nil` takes it to 0 failures across 8 combined `-race` runs. The underlying executor bug is pre-existing and not yours — I reproduced the identical failure on the merge base using `divide`-by-zero, at 47/2000 — so the ask here is just to stop the tests tripping it. I'll file the executor issue separately against `executor.go:521`. -- 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]
