Repository navigation
docs(parquet): add SAFETY comments for build_unchecked in array readers - #12119
Merged
Rich-T-kid merged 2 commits intoNov 9, 2026
Merged
Conversation
Rich-T-kid
approved these changes
Nov 8, 2026
Comment on lines
-205
to
-212
| debug_assert_eq!( | ||
| record_data.buffer.len(), | ||
| len * self.byte_length, | ||
| "fixed-len byte array buffer size mismatch: {} bytes for {} elements of size {}", | ||
| record_data.buffer.len(), | ||
| len, | ||
| self.byte_length, | ||
| ); |
Contributor
There was a problem hiding this comment.
nice, I think its better to replace this with actual validation.
| len, | ||
| self.byte_length, | ||
| ); | ||
| if record_data.buffer.len() != len * self.byte_length { |
Contributor
There was a problem hiding this comment.
can we actually make this a checked_mul just to avoid weird behavoir in the unlikely case it overflows
Contributor
|
❤️ |
Contributor
|
for the sake of getting this over line I updated the PR. thank you @EaswarKilari |
Contributor
Author
|
thank you @Rich-T-kid . Sorry for the delay (was busy in some other work). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
A few
build_uncheckedcalls in the parquet array readers had no comment explaining why they're safe.What changes are included in this PR?
SAFETYcomments in the list, fixed size list and fixed len byte array readers.debug_assert, so it's now a real check that returns an error in release builds too.I left
OffsetBuffer::into_arrayout for now, since #11144 is fixing the UTF-8 issue there (#11140). Happy to add the comment once that's merged.Are these changes tested?
Covered by existing tests. The new check shouldn't trigger for valid files.
Are there any significant user-facing changes?
No.