joechenrh commented on code in PR #880:
URL: https://github.com/apache/arrow-go/pull/880#discussion_r3563094765


##########
parquet/file/page_reader.go:
##########
@@ -91,6 +99,43 @@ func (p *page) Data() []byte              { return 
p.buf.Bytes() }
 func (p *page) NumValues() int32          { return p.nvals }
 func (p *page) Encoding() format.Encoding { return p.encoding }
 
+// dataPageBase holds the members shared by DataPageV1/V2 but not dictionary
+// pages: the rep/def level buffer accessor and, in streaming mode, the
+// incremental value source.
+type dataPageBase struct {
+       page
+
+       // valueBuf is only set for streaming mode to read values incrementally.
+       // In such mode, buf only holds the rep+def level region.
+       valueBuf streaming.ValueBuffer
+       // clip bounds a read's batch to keep one Decode's aliased values near 
the buffer
+       // size; 0 for non-streaming pages.
+       clip int64
+}
+
+func (d *dataPageBase) valueBuffer() streaming.ValueBuffer { return d.valueBuf 
}
+func (d *dataPageBase) levelBuffer() []byte                { return 
d.buf.Bytes() }
+
+func (d *dataPageBase) valueClip() int64 { return d.clip }
+
+func (d *dataPageBase) setStreamingValues(mem memory.Allocator, 
lenUncompressed int, levelBuf []byte, valReader, rawSrc io.Reader, closer 
io.Closer) {
+       d.buf = memory.NewBufferBytes(levelBuf)

Review Comment:
   Fixed in 
https://github.com/apache/arrow-go/commit/7529291d305c17eb9d8435d0cfd0ca7080d4bc94
 — since this fix is not large, so I think we can add it in this PR. The 
rep/def level region now reads into an allocator-backed buffer so 
`DataPage.Release` frees it and checked/custom allocators account for it.
   



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