[PR] fix(arrow/scalar): implement Release()/Retain() on *scalar.Extension [arrow-go]

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade opened a new pull request, #851:
URL: https://github.com/apache/arrow-go/pull/851

   ### Rationale for this change
   
   Closes #827.
   
   `*scalar.Extension` implements `value()`, `equals()`, `Validate()`, `ValidateFull()`, `CastTo()`, and `String()`, but neither `Release()` nor `Retain()`.
   
   `compute.ScalarDatum.Release()` only forwards `Release()` when the wrapped scalar satisfies the unexported `releasable` interface (`arrow/compute/datum.go`):
   
   ```go
   func (d *ScalarDatum) Release() {
   	if v, ok := d.Value.(releasable); ok {
   		v.Release()
   	}
   }
   ```
   
   Because `*scalar.Extension` doesn't satisfy that interface, the type assertion fails and the inner storage scalar is never released — even though the storage scalar (e.g. `*scalar.FixedSizeList` backing the interval extension types) carries buffer-bearing arrays. Any code path that produces an extension scalar and releases it through a `ScalarDatum` leaks those buffers. Concretely this affects the `*types.IntervalYearToMonthType` / `*types.IntervalDayType` paths in `arrow/compute/exprs`.
   
   `*scalar.Extension` was the lone outlier among buffer-holding scalar types: `*scalar.Binary`, `*scalar.List`, `*scalar.Struct`, `*scalar.Dictionary`, `*scalar.SparseUnion`, `*scalar.DenseUnion`, and `*scalar.RunEndEncoded` all already delegate `Retain`/`Release` to their inner storage.
   
   ### What changes are included in this PR?
   
   1. **`*scalar.Extension` now implements `Retain()` and `Release()`**, delegating to the storage scalar when it satisfies the `Releasable` interface (`arrow/scalar/scalar.go`). This mirrors `*scalar.Binary` and the nested scalar types:
   
      ```go
      func (s *Extension) Retain() {
          if r, ok := s.Value.(Releasable); ok {
              r.Retain()
          }
      }
   
      func (s *Extension) Release() {
          if r, ok := s.Value.(Releasable); ok {
              r.Release()
          }
      }
      ```
   
      This is purely additive:
      - Storage scalars that don't implement `Releasable` (e.g. primitive numeric storage) are no-ops, matching today's behavior.
      - Storage scalars that do (e.g. `*scalar.FixedSizeList` for the interval extension types) are now properly released.
      - The `Retain()` half restores symmetry so a caller taking ownership of an extension scalar can bump the storage refcount.
   
   2. **`TestLiteralToDatumIntervalYearToMonth` is switched to `memory.NewCheckedAllocator` with `defer mem.AssertSize(t, 0)`** (`arrow/compute/exprs/exec_internal_test.go`). That test previously used `memory.DefaultAllocator` specifically to dodge this leak and carried a comment deferring the fix to this issue; the workaround comment is removed.
   
   ### Are these changes tested?
   
   Yes. `TestLiteralToDatumIntervalYearToMonth` now asserts zero outstanding allocations via a checked allocator. Verified that it **fails without** the scalar change (`invalid memory size exp=0, got=384`, with leak traces pointing at the extension scalar's storage) and **passes with** it. The full `arrow/scalar/...` and `arrow/compute/exprs/...` suites pass, and `go vet` is clean.
   
   ### Are there any user-facing changes?
   
   No breaking changes. Adding `Retain()`/`Release()` methods to an exported type is non-breaking in Go. The only behavioral change is the intended one: buffers held by an extension scalar's storage are now released when the scalar (or a `ScalarDatum` wrapping it) is released, instead of leaking.
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(arrow/scalar): implement Release()/Retain() on *scalar.Extension [arrow-go]

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade merged PR #851:
URL: https://github.com/apache/arrow-go/pull/851


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org