This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new e7b149e3 fix(arrow/array): clean up partial table construction (#1077)
e7b149e3 is described below
commit e7b149e3d1de47aa88ae47669ea889c40a25bea9
Author: Minh Vu <[email protected]>
AuthorDate: Wed Aug 5 20:18:17 2026 +0200
fix(arrow/array): clean up partial table construction (#1077)
### Rationale for this change
NewTableFromSlice creates each column before validating the completed
table. Its cleanup was installed only after the construction loop, so a
panic from a later column left every earlier column holding an extra
reference to its arrays.
A two-column checked-allocator repro leaves 128 bytes allocated on main
when the second column has the wrong datatype.
### What changes are included in this PR?
Register deferred cleanup before column construction, track how many
columns have completed, and disable cleanup only after validation
succeeds. Failure unwinds naturally with the original panic after
releasing exactly the completed columns. The successful construction
path and ownership contract are unchanged.
### Are these changes tested?
Yes. The regression test constructs the first column, triggers
ErrInvalid from the second, recovers the expected panic, and verifies
that releasing the caller-owned arrays returns allocator usage to zero.
- go test ./arrow/array
- go test -race ./arrow/array -run
'^TestTableFromSliceReleasesPartialColumnsOnPanic$' -count=1
### Are there any user-facing changes?
No API changes. Failed table construction no longer retains arrays from
earlier columns.
---
arrow/array/table.go | 22 ++++++++++++----------
arrow/array/table_test.go | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/arrow/array/table.go b/arrow/array/table.go
index 92d265e0..c8cd7ed9 100644
--- a/arrow/array/table.go
+++ b/arrow/array/table.go
@@ -142,11 +142,22 @@ func NewTableFromSlice(schema *arrow.Schema, data
[][]arrow.Array) arrow.Table {
}
cols := make([]arrow.Column, schema.NumFields())
+ constructed := 0
+ complete := false
+ defer func() {
+ if !complete {
+ for i := 0; i < constructed; i++ {
+ cols[i].Release()
+ }
+ }
+ }()
+
for i, arrs := range data {
field := schema.Field(i)
chunked := arrow.NewChunked(field.Type, arrs)
cols[i] = *arrow.NewColumn(field, chunked)
chunked.Release()
+ constructed++
}
var rows int64
@@ -161,18 +172,9 @@ func NewTableFromSlice(schema *arrow.Schema, data
[][]arrow.Array) arrow.Table {
}
tbl.refCount.Add(1)
- defer func() {
- if r := recover(); r != nil {
- // if validate panics, let's release the columns
- // so that we don't leak them, then propagate the panic
- for _, c := range cols {
- c.Release()
- }
- panic(r)
- }
- }()
// validate the table and its constituents.
tbl.validate()
+ complete = true
return &tbl
}
diff --git a/arrow/array/table_test.go b/arrow/array/table_test.go
index 4720f606..94c308b7 100644
--- a/arrow/array/table_test.go
+++ b/arrow/array/table_test.go
@@ -123,6 +123,42 @@ func TestTableFromSliceWithoutColumns(t *testing.T) {
}
}
+func TestTableFromSliceReleasesPartialColumnsOnPanic(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+ defer mem.AssertSize(t, 0)
+
+ ib := array.NewInt32Builder(mem)
+ ib.Append(1)
+ first := ib.NewArray()
+ ib.Release()
+
+ lb := array.NewInt64Builder(mem)
+ lb.Append(2)
+ second := lb.NewArray()
+ lb.Release()
+
+ schema := arrow.NewSchema([]arrow.Field{
+ {Name: "first", Type: arrow.PrimitiveTypes.Int32},
+ {Name: "second", Type: arrow.PrimitiveTypes.Int32},
+ }, nil)
+ func() {
+ defer func() {
+ r := recover()
+ if r == nil {
+ t.Fatal("expected NewTableFromSlice to panic
for mismatched types")
+ }
+ err, ok := r.(error)
+ if !ok || !errors.Is(err, arrow.ErrInvalid) {
+ t.Fatalf("expected ErrInvalid, got %v", r)
+ }
+ }()
+ array.NewTableFromSlice(schema, [][]arrow.Array{{first},
{second}})
+ }()
+
+ first.Release()
+ second.Release()
+}
+
func TestTableFromRecordsWithoutColumns(t *testing.T) {
schema := arrow.NewSchema(nil, nil)
records := []arrow.RecordBatch{
