You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "joechenrh (via GitHub)" <gi...@apache.org> on 2025/03/04 13:33:48 UTC
[PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
joechenrh opened a new pull request, #302:
URL: https://github.com/apache/arrow-go/pull/302
### Rationale for this change
When we create a reader with `BufferedStreamEnabled = false`, we have to first create a `data` slice.
https://github.com/apache/arrow-go/blob/efecae3596e6734ce8244a5ed1befdcff09884df/parquet/reader_properties.go#L76-L91
And we will create another slice with same size in the `NewBufferedReader` again, which is redundant.
https://github.com/apache/arrow-go/blob/efecae3596e6734ce8244a5ed1befdcff09884df/internal/utils/buf_reader.go#L45-L51
### What changes are included in this PR?
- Add a new struct `byteReader`, which is a wrapper of `bytes.NewReader`
- Remove `BufferSize()` method from `BufferedReader` interface
### Are these changes tested?
`BufferedReader` created from `bytes.NewReader` in tests are all replaced by new function `NewByteReader`.
### Are there any user-facing changes?
No.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #302:
URL: https://github.com/apache/arrow-go/pull/302#discussion_r1979738429
##########
parquet/file/page_reader.go:
##########
@@ -533,7 +533,7 @@ func (p *serializedPageReader) GetDictionaryPage() (*DictionaryPage, error) {
hdr := format.NewPageHeader()
rd := utils.NewBufferedReader(
io.NewSectionReader(p.r.Outer(), p.dictOffset-p.baseOffset, p.dataOffset-p.baseOffset),
- p.r.BufferSize())
+ int(parquet.DefaultBufSize))
Review Comment:
This would remove our respecting of the BufferSize property in the reader properties, we shouldn't be removing the `BufferSize()` method
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "joechenrh (via GitHub)" <gi...@apache.org>.
joechenrh commented on code in PR #302:
URL: https://github.com/apache/arrow-go/pull/302#discussion_r1980511391
##########
parquet/file/page_reader.go:
##########
@@ -533,7 +533,7 @@ func (p *serializedPageReader) GetDictionaryPage() (*DictionaryPage, error) {
hdr := format.NewPageHeader()
rd := utils.NewBufferedReader(
io.NewSectionReader(p.r.Outer(), p.dictOffset-p.baseOffset, p.dataOffset-p.baseOffset),
- p.r.BufferSize())
+ int(parquet.DefaultBufSize))
Review Comment:
So how about change it to:
```go
readBufSize := min(p.dataOffset-p.baseOffset, p.r.BufferSize())
rd := utils.NewBufferedReader(
io.NewSectionReader(p.r.Outer(), p.dictOffset-p.baseOffset, p.dataOffset-p.baseOffset),
readBufSize)
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #302:
URL: https://github.com/apache/arrow-go/pull/302#issuecomment-2705784579
Done!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #302:
URL: https://github.com/apache/arrow-go/pull/302
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #302:
URL: https://github.com/apache/arrow-go/pull/302#discussion_r1981912898
##########
parquet/file/page_reader.go:
##########
@@ -533,7 +533,7 @@ func (p *serializedPageReader) GetDictionaryPage() (*DictionaryPage, error) {
hdr := format.NewPageHeader()
rd := utils.NewBufferedReader(
io.NewSectionReader(p.r.Outer(), p.dictOffset-p.baseOffset, p.dataOffset-p.baseOffset),
- p.r.BufferSize())
+ int(parquet.DefaultBufSize))
Review Comment:
that works
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(parquet): add byte buffer when disable buffered stream [arrow-go]
Posted by "joechenrh (via GitHub)" <gi...@apache.org>.
joechenrh commented on PR #302:
URL: https://github.com/apache/arrow-go/pull/302#issuecomment-2705587155
Can I manually rerun the CI?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org