apache / apache/arrow-go

[Compute] NewDatumWithoutOwning returns an ordinary Datum, so Release() on it frees the caller's array with no error; its doc comment is also truncated

Open
#1,298 0 comments 0 reactions 0 assignees View on GitHub
good-first-issue Type: enhancement Type: usage
Dominant language
Assembly
Stars
404
Forks
145
Avg merge
2d 4h
Merged PRs (30d)
87

Description

**Describe the bug**

`compute.NewDatumWithoutOwning` returns the same `Datum` implementation as `compute.NewDatum`, so
calling `Release()` on it compiles and runs like any other datum. But the datum did not retain the
value, so that `Release()` drops the reference the caller's array still depends on: the allocator
reports the buffers freed while the array is in scope and readable, and the array's own later
`Release()` does not panic. The doc comment says "should not have Release called on it", and ends
with a truncated sentence: "For the most part this is just a convenience function.+-" (datum.go
line 278).

With the Go allocator this is silent, because the memory stays alive under the garbage collector.
With a C allocator, or when the buffers were imported from C through the cdata package, the same
call frees memory under a live array.

Go 1.27.1, macOS 26.6 (arm64), arrow-go v18.7.0; identical on v18.8.0-rc1.

```go
mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
b := array.NewInt64Builder(mem)
b.AppendValues([]int64{1, 2, 3}, nil)
arr := b.NewArray().(*array.Int64)
b.Release()
fmt.Println("after build:", mem.CurrentAlloc(), "bytes") // 128 bytes

d := compute.NewDatumWithoutOwning(arr)
d.Release() // compiles; the doc says not to
fmt.Println("after datum.Release():", mem.CurrentAlloc(), "bytes, arr.Len()", arr.Len(), arr.Int64Values())
arr.Release() // no panic
```

Output:

```
after build: 128 bytes
after datum.Release(): 0 bytes, arr.Len() 3 [1 2 3]
```

For contrast, `compute.NewDatum(arr)` followed by `Release()` leaves 128 bytes allocated until
`arr.Release()`, as expected.

**Expected behavior**

One of: a non-owning datum whose `Release()` is a no-op (it owns nothing); or a distinct type that
does not expose `Release`; or, at least, a panic on the over-release. And the doc comment completed.

**Component(s)**

Go, Compute

Contributor guide

Open the contributing guide

Research direction

Start in datum.go around line 278 and compare compute.NewDatumWithoutOwning with compute.NewDatum, then trace how Release handles ownership and allocator references. Decide which expected ownership behavior is appropriate, add coverage for the live-array case, and complete the truncated documentation; done means the caller's array remains safely owned and the documented API matches the behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
data
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.