[ 
https://issues.apache.org/jira/browse/ARROW-18274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17630077#comment-17630077
 ] 

Matthew Topol commented on ARROW-18274:
---------------------------------------

Okay. 

I'll leave the PR up until tomorrow to give others a chance to comment on it 
and if there's no objections by morning I'll merge it in (and then you can bump 
to v11). 

After that, tomorrow i'll send an email to the ML proposing a cherry-pick + 
patch release of v10.0.1 for that.

> [Go] Sparse union of structs is buggy
> -------------------------------------
>
>                 Key: ARROW-18274
>                 URL: https://issues.apache.org/jira/browse/ARROW-18274
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: Go
>    Affects Versions: 10.0.0
>            Reporter: Laurent Querel
>            Assignee: Matthew Topol
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> There is a bug with union of structs in V10.
> The first unit test crash with a panic (i.e. invalid memory address or nil 
> pointer dereference). The second test works as expected.
>  
> {code:go}
> func TestDoesNotWork(t *testing.T) {
>    dt1 := arrow.SparseUnionOf([]arrow.Field{
>       {Name: "c", Type: arrow2.DictU16String},
>    }, []arrow.UnionTypeCode{0})
>    dt2 := arrow.StructOf(
>       arrow.Field{Name: "b", Type: dt1},
>    )
>    dt3 := arrow.SparseUnionOf([]arrow.Field{
>       {Name: "a", Type: dt2},
>    }, []arrow.UnionTypeCode{0})
>    pool := memory.NewGoAllocator()
>    builder := array.NewSparseUnionBuilder(pool, dt3)
>    defer builder.Release()
>    arr := builder.NewArray()
>    defer arr.Release()
>    assert.Equal(t, 0, arr.Len())
> }
> func TestWorksAsExpected(t *testing.T) {
>    dt1 := arrow.SparseUnionOf([]arrow.Field{
>       {Name: "c", Type: &arrow.DictionaryType{
>          IndexType: arrow.PrimitiveTypes.Uint16,
>          ValueType: arrow.BinaryTypes.String,
>          Ordered:   false,
>       }},
>    }, []arrow.UnionTypeCode{0})
>    dt2 := arrow.SparseUnionOf([]arrow.Field{
>       {Name: "a", Type: dt1},
>    }, []arrow.UnionTypeCode{0})
>    pool := memory.NewGoAllocator()
>    builder := array.NewSparseUnionBuilder(pool, dt2)
>    defer builder.Release()
>    arr := builder.NewArray()
>    defer arr.Release()
>    assert.Equal(t, 0, arr.Len())
> } {code}
>  
> *Analysis:*
>  - The `NewSparseUnionBuilder` calls the builders for each variant and also 
> calls defer builder.Release. 
>  - The Struct Release method calls the Release methods of every field even if 
> the refCount is not 0, so the Release method of the second union is called 
> followed by the Release method of the dictionary. 
>  - Although, the union builder is returned without error, the builder is not 
> usable.
>  - This bug doesn't happen with 2 nested unions. As the internal counter is 
> properly tested.
>  
> First, I don't understand why the Release method of each variant is called 
> right after the Union constructor is created. I also don't understand why the 
> Release method of the structure calls the Release method of each field 
> regardless of the value of the internal refCount. This looks like a bug to 
> me, but I'm not quite sure yet what the right way to fix it will be.
>  
> Any idea?



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to