hcrosse opened a new issue, #1450:
URL: https://github.com/apache/arrow-go/issues/1450
### Describe the bug, including details regarding any error messages,
version, and platform.
`ExecuteScalarExpression` leaks the referenced column's buffers when a
top-level field reference has the wrong type. Returning an error leaves an
extra reference to the column, so releasing the input does not free its buffers.
I reproduced this on main at `22c9f26d` on Linux amd64 and on v18.8.0 on
macOS arm64. An expression that expects `utf8`, executed on a two-row `int32`
column, returns `arrow.ErrInvalid` but leaves 128 bytes allocated after the
input is released.
**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 .`:
```go
// Leak in exprs.ExecuteScalarExpression when a top-level field reference's
// type does not match the input column.
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)
// The expression is built against a schema where "col" is utf8 ...
exprSchema := arrow.NewSchema([]arrow.Field{{Name: "col", Type:
arrow.BinaryTypes.String, Nullable: true}}, nil)
// ... but executed against data where "col" is int32.
dataSchema := arrow.NewSchema([]arrow.Field{{Name: "col", Type:
arrow.PrimitiveTypes.Int32, Nullable: true}}, nil)
rec, _, err := array.RecordFromJSON(mem, dataSchema,
strings.NewReader(`[{"col": 1}, {"col": 2}]`))
if err != nil {
panic(err)
}
bldr := exprs.NewExprBuilder(exprs.NewDefaultExtensionSet())
if err := bldr.SetInputSchema(exprSchema); err != nil {
panic(err)
}
e, err := bldr.FieldRef("col").BuildExpr()
if err != nil {
panic(err)
}
ctx := compute.WithAllocator(context.Background(), mem)
result, err := exprs.ExecuteScalarExpression(ctx, dataSchema, e,
compute.NewDatumWithoutOwning(rec))
fmt.Printf("result=%v err=%v\n", result, err)
if result != nil {
result.Release()
}
rec.Release()
fmt.Printf("bytes still allocated after releasing everything: %d\n",
mem.CurrentAlloc())
}
```
The final line on main is:
```text
bytes still allocated after releasing everything: 128
```
**Expected behavior**
The type mismatch should still return `arrow.ErrInvalid`, but the allocator
should report zero bytes after the input is released. With a patch that retains
the column only after the type check, the same program reports zero.
**Cause**
`execFieldRef` retains the column through `compute.NewDatum(param)` before
checking its type:
https://github.com/apache/arrow-go/blob/22c9f26df0abbc2066f5d9ae45f31720d29a4300/arrow/compute/exprs/exec.go#L518-L536
When the check fails, it returns without releasing that datum. This affects
any top-level column type mismatch that reaches this error branch, not only
`int32` versus `utf8`. Only the top-level path takes this reference. A nested
reference returns a value borrowed from its parent, so releasing `out` on the
error path would release a reference `execFieldRef` does not own. Retaining the
column only after the type check avoids that.
https://github.com/apache/arrow-go/pull/1449 changes the same type check to
accept dictionary columns and does not change this reference counting.
I have a patch with tests 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]
