zeroshade commented on code in PR #924:
URL: https://github.com/apache/arrow-go/pull/924#discussion_r3572941433


##########
parquet/pqarrow/column_readers.go:
##########
@@ -162,13 +162,23 @@ func (lr *leafReader) clearOut() (out *arrow.Chunked) {
 
 func (lr *leafReader) Field() *arrow.Field { return lr.field }
 
+func (lr *leafReader) setPageReader(pr file.PageReader) error {
+       rr, ok := lr.recordRdr.(file.RecordReaderWithError)
+       if !ok {
+               return errors.New("record reader does not support 
error-returning page reader replacement")
+       }
+       return rr.SetPageReaderWithError(pr)

Review Comment:
   If `SetPageReaderWithError(pr)` returns an error, the freshly acquired `pr` 
is neither installed nor closed. Both call sites (`SeekToRow` and 
`nextRowGroup`) obtain a new `PageReader` immediately before calling this 
helper, so a replacement failure leaks the reader and its buffers. Please close 
`pr` before returning the error here (or transfer ownership back and close it 
at each call site).



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