-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[Parquet] Use u64 for SerializedPageReaderState.offset & remaining_bytes, instead of usize
#7918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Parquet] Use u64 for SerializedPageReaderState.offset & remaining_bytes, instead of usize
#7918
Conversation
Signed-off-by: Jigao Luo <[email protected]>
Signed-off-by: Jigao Luo <[email protected]>
u64 for SerializedPageReaderState.offset & remaining_bytes, instead of usizeu64 for SerializedPageReaderState.offset & remaining_bytes, instead of usize
Signed-off-by: Jigao Luo <[email protected]>
XiangpengHao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @JigaoLuo this looks good to me! left minor optional doc change.
Signed-off-by: Jigao Luo <[email protected]>
|
Thank you @XiangpengHao for reviewing. I have updated the doc. |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JigaoLuo and @XiangpengHao -- this makes sense to me
@etseidl or @jhorstmann do you have any concerns about this change?
|
Looks good to me, thanks! |
|
Thanks again @JigaoLuo and thanks @mbrobbel and @jhorstmann |
|
Thank you to all the reviewers |
Which issue does this PR close?
SerializedPageReaderState.offset&remaining_bytesbeu64instead ofusize? #7910Rationale for this change
There is a copy from my issue page:
arrow-rs/parquet/src/file/serialized_reader.rs
Lines 471 to 472 in 2be261b
What changes are included in this PR?
This PR does type changes only for
SerializedPageReaderState.offset&remaining_bytesAre these changes tested?
I can pass with local unit tests via
cargo test -p parquetAre there any user-facing changes?
No