fix(parquet/pqarrow): release temporary extension arrays - #1060
Open
fallintoplace wants to merge 3 commits into
Open
fix(parquet/pqarrow): release temporary extension arrays#1060fallintoplace wants to merge 3 commits into
fallintoplace wants to merge 3 commits into
Conversation
serramatutu
suggested changes
Jul 30, 2026
| extType := er.fieldWithExt.Type.(arrow.ExtensionType) | ||
|
|
||
| newChunks := make([]arrow.Array, len(chkd.Chunks())) | ||
| defer func() { |
Contributor
There was a problem hiding this comment.
Can we add a unit test to repro this using the checked allocator?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
extensionReader.BuildArraycreates extension arrays and passes them toarrow.NewChunked, which retains each chunk. The original references were not released, leaking one array reference per output chunk. A failure after some chunks have already been wrapped also leaked those partial results.What changes are included in this PR?
Defer cleanup of every constructed extension array. On success,
arrow.NewChunkedkeeps its retained references; on failure, already-constructed chunks are released during unwinding.Are these changes tested?
Yes. A checked-allocator regression test makes the second extension chunk fail after the first has been wrapped. Reverting the cleanup leaves the first chunk's buffers allocated; the fix returns allocator usage to zero.
go test ./parquet/pqarrow -run TestExtensionReaderBuildArrayReleasesPartialChunksOnPanicAre there any user-facing changes?
No API changes. Extension-column reads no longer retain temporary or partially constructed array references.