joellubi commented on code in PR #41698:
URL: https://github.com/apache/arrow/pull/41698#discussion_r1605237421


##########
go/parquet/file/page_writer.go:
##########
@@ -451,6 +451,7 @@ func (bw *bufferedPageWriter) Close(hasDict, fallback bool) 
error {
 
        buf := bw.inMemSink.Finish()
        defer buf.Release()
+       bw.inMemSink.Release()

Review Comment:
   Yeah I think that would be cleaner. The culprit is the call to 
`BufferWriter.Reserve(0)` to reset the buffer. It's not actually possible to 
reserve 0 bytes currently, the minimum capacity is 256 bytes due to the line 
`newCap := utils.Max(b.buffer.Cap()+b.offset, 256)`.
   
   Setting this to simply `newCap := b.buffer.Cap()+b.offset` should fix it, 
but I wasn't sure if there were existing cases where the minimum buffer size 
was useful. We could also treat `BufferWriter.Reset(0)` as a special case and 
keep the buffer empty, whereas nonzero values would get rounded up to 256 since 
the intent is clearly to allocate some memory in that case. What do you think?



-- 
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]

Reply via email to