You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ze...@apache.org on 2026/08/11 14:16:08 UTC

(arrow-go) branch main updated: fix(arrow/scalar): release partial children after failed struct conversion (#1114)

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 db4317e1 fix(arrow/scalar): release partial children after failed struct conversion (#1114)
db4317e1 is described below

commit db4317e1b868475e0b11aa08e033a7f861cd62a5
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Tue Aug 11 16:16:00 2026 +0200

    fix(arrow/scalar): release partial children after failed struct conversion (#1114)
    
    ### Rationale for this change
    
    ToScalar creates child scalars incrementally. If a later Struct field
    fails, earlier array-backed children can remain retained after the
    conversion returns an error.
    
    ### What changes are included in this PR?
    
    Release partial children on failure while keeping the existing ownership
    transfer for a completed struct scalar. Add checked-allocator coverage
    for the failed conversion.
    
    ### Are these changes tested?
    
    - `go test ./arrow/scalar`
    
    ### Are there any user-facing changes?
    
    No API changes. This corrects the reported behavior while preserving the
    existing ownership and compatibility contracts.
---
 arrow/scalar/parse.go       | 18 +++++++++++++++++-
 arrow/scalar/scalar_test.go | 16 ++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arrow/scalar/parse.go b/arrow/scalar/parse.go
index 69108e8f..41de7717 100644
--- a/arrow/scalar/parse.go
+++ b/arrow/scalar/parse.go
@@ -140,6 +140,17 @@ func ToScalar(val interface{}, mem memory.Allocator) (Scalar, error) {
 	case reflect.Struct:
 		scalars := make([]Scalar, 0, v.Type().NumField())
 		fields := make([]string, 0, v.Type().NumField())
+		success := false
+		defer func() {
+			if success {
+				return
+			}
+			for _, child := range scalars {
+				if releasable, ok := child.(Releasable); ok {
+					releasable.Release()
+				}
+			}
+		}()
 		for i := 0; i < v.Type().NumField(); i++ {
 			fld := v.Type().Field(i)
 			tag := fld.Tag.Get("compute")
@@ -162,7 +173,12 @@ func ToScalar(val interface{}, mem memory.Allocator) (Scalar, error) {
 			fields = append(fields, "_type_name")
 		}
 
-		return NewStructScalarWithNames(scalars, fields)
+		out, err := NewStructScalarWithNames(scalars, fields)
+		if err != nil {
+			return nil, err
+		}
+		success = true
+		return out, nil
 	case reflect.Slice:
 		return createListScalar(v, mem)
 	default:
diff --git a/arrow/scalar/scalar_test.go b/arrow/scalar/scalar_test.go
index 24cb7ba0..392019d7 100644
--- a/arrow/scalar/scalar_test.go
+++ b/arrow/scalar/scalar_test.go
@@ -1410,6 +1410,11 @@ func (v valueFromScalarTarget) FromStructScalar(*scalar.Struct) error {
 	return nil
 }
 
+type PartialScalarTest struct {
+	Good []string
+	Bad  []complex64
+}
+
 func TestToScalar(t *testing.T) {
 	ot := &OptionValTest{ToType: arrow.BinaryTypes.String, Allow: true}
 	sc, err := scalar.ToScalar(ot, memory.DefaultAllocator)
@@ -1515,6 +1520,17 @@ func TestFromScalarMetadataDoesNotPrependEmptyEntries(t *testing.T) {
 	assert.Equal(t, meta.Values(), out.FieldMeta[0].Values())
 }
 
+func TestToScalarReleasesPartialStructOnError(t *testing.T) {
+	mem := memory.NewCheckedAllocator(memory.NewGoAllocator())
+	defer mem.AssertSize(t, 0)
+
+	_, err := scalar.ToScalar(PartialScalarTest{
+		Good: []string{"retained before error"},
+		Bad:  []complex64{complex(1, 2)},
+	}, mem)
+	require.Error(t, err)
+}
+
 var dictIndexTypes = []arrow.DataType{
 	arrow.PrimitiveTypes.Int8,
 	arrow.PrimitiveTypes.Uint8,