hcrosse opened a new issue, #1451:
URL: https://github.com/apache/arrow-go/issues/1451
### Describe the bug, including details regarding any error messages,
version, and platform.
`ExecuteScalarExpression` returns a nested struct field without retaining
it. Releasing the input frees the result's buffers while the caller still holds
the result, and releasing the result with `-tags assert` panics with `too many
releases`.
I reproduced this on main at `22c9f26d` and on v18.8.0, on macOS arm64, with
`FieldRefList("s", "a")` over a two-row record. Without assertions, the
`CheckedAllocator` reports zero bytes after the input is released, down from
192, even though the result is still held.
**Reproduction**
Run it in a module that requires `github.com/apache/arrow-go/v18 v18.8.0`,
or in a subdirectory of a checkout at `22c9f26d`, with `go run .` and then `go
run -tags assert .`:
```go
package main
import (
"context"
"fmt"
"strings"
"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/compute/exprs"
"github.com/apache/arrow-go/v18/arrow/memory"
)
func main() {
mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
sc := arrow.NewSchema([]arrow.Field{{Name: "s", Type: arrow.StructOf(
arrow.Field{Name: "a", Type: arrow.PrimitiveTypes.Int32,
Nullable: true}), Nullable: true}}, nil)
rec, _, err := array.RecordFromJSON(mem, sc, strings.NewReader(`[{"s":
{"a": 1}}, {"s": {"a": 2}}]`))
if err != nil {
panic(err)
}
bldr := exprs.NewExprBuilder(exprs.NewDefaultExtensionSet())
if err := bldr.SetInputSchema(sc); err != nil {
panic(err)
}
e, err := bldr.FieldRefList("s", "a").BuildExpr()
if err != nil {
panic(err)
}
ctx := compute.WithAllocator(context.Background(), mem)
result, err := exprs.ExecuteScalarExpression(ctx, sc, e,
compute.NewDatumWithoutOwning(rec))
if err != nil {
panic(err)
}
fmt.Printf("result=%v bytes allocated=%d\n", result, mem.CurrentAlloc())
rec.Release()
// The result still references s.a, so its buffers should still be
allocated.
fmt.Printf("after rec.Release, result still held: bytes
allocated=%d\n", mem.CurrentAlloc())
result.Release()
fmt.Printf("after result.Release: bytes allocated=%d\n",
mem.CurrentAlloc())
}
```
Output without assertions:
```text
result=Array:{int32} bytes allocated=192
after rec.Release, result still held: bytes allocated=0
after result.Release: bytes allocated=0
```
With `-tags assert`, the program instead panics at `result.Release()` with
`too many releases`.
**Expected behavior**
The result should keep its buffers alive until the caller releases it,
independently of the input record. Releasing both should not panic.
**Cause and scope**
`GetReferencedValue` returns the struct child borrowed from its input,
without retaining it:
https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/arrow/compute/exprs/field_refs.go#L205-L216
`execFieldRef` returns that datum without taking a reference. Its callers
release the result, including the cast and scalar-function paths. List-element
and map-value references on scalar inputs return new scalars instead of
borrowed values, so a fix cannot retain every result:
-
https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/arrow/compute/exprs/exec.go#L558-L562
releases the cast argument.
-
https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/arrow/compute/exprs/exec.go#L590-L594
releases each scalar-function argument.
Top-level field references take their own reference and are not affected.
This is separate from the type-mismatch leak in the same function, reported in
https://github.com/apache/arrow-go/issues/1450. The reproduction exercises a
successful nested field reference directly through `ExecuteScalarExpression`,
not a cast or scalar function.
A filter on a nested column takes the same path. On v18.8.0, evaluating
`equal(s.a, 1)` and then releasing the record panics with `too many releases`
under `-tags assert`, because the scalar function releases the record's own
child.
I have a patch with tests, built on the fix for that leak, and can open a PR.
### Component(s)
Other
--
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]
