zeroshade commented on code in PR #1121:
URL: https://github.com/apache/arrow-go/pull/1121#discussion_r3752061123
##########
arrow/csv/reader_test.go:
##########
@@ -219,6 +219,32 @@ func TestCSVReadInvalidFields(t *testing.T) {
}
}
+func TestFixedSizeBinaryParseErrorAppendsNull(t *testing.T) {
+ schema := arrow.NewSchema(
+ []arrow.Field{
+ {Name: "id", Type: arrow.PrimitiveTypes.Int64},
+ {Name: "value", Type:
&arrow.FixedSizeBinaryType{ByteWidth: 3}},
+ {Name: "name", Type: arrow.BinaryTypes.String},
+ },
+ nil,
+ )
+ r := csv.NewReader(strings.NewReader("1,AQ==,name\n"), schema,
csv.WithHeader(false))
+ defer r.Release()
+
+ require.True(t, r.Next())
+ require.ErrorIs(t, r.Err(), arrow.ErrInvalid)
+
+ record := r.RecordBatch()
+ defer record.Release()
Review Comment:
This double-releases the record batch, which is what's failing CI.
`RecordBatch()` returns `r.cur` directly — a borrowed reference, no
`Retain()`. The reader owns it ("valid until the next call to Next"), and
`Reader.Release()` releases `r.cur` itself. No other test in this file releases
the batch it gets back from `RecordBatch()`.
Defers run LIFO, so `record.Release()` fires first and drops the refcount to
zero, then `r.Release()` releases it a second time and trips
`debug.Assert(rec.refCount.Load() > 0, "too many releases")` in
`array/record.go:244`. The stack lands on line 246, the closing brace, which is
where the deferred calls execute.
CI runs with `-tags assert,test,ccalloc`, which is why this fires there but
not under a plain `go test ./arrow/csv` — that's why the command in the PR
description passes locally. Worth running with `-tags assert` when a test
touches refcounts.
Fix: drop this line. (Or `record.Retain()` before it, but simply removing it
matches the rest of the file.)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
