alamb commented on code in PR #9910:
URL: https://github.com/apache/arrow-rs/pull/9910#discussion_r3190925265
##########
arrow-array/src/array/fixed_size_binary_array.rs:
##########
@@ -88,11 +88,19 @@ use std::sync::Arc;
///
#[derive(Clone)]
pub struct FixedSizeBinaryArray {
- data_type: DataType, // Must be DataType::FixedSizeBinary(value_length)
+ /// Must be DataType::FixedSizeBinary(value_length)
+ data_type: DataType,
+ /// `len` values, each `value_size` bytes
value_data: Buffer,
+ /// Optional Null Buffer
nulls: Option<NullBuffer>,
+ /// Number of elements in the array
len: usize,
- value_length: i32,
+ /// size of each element, validated to fit in a positive i32
+ ///
+ /// note: Arrow stores `value_len` using i32. This implementation stores it
+ /// as a usize to ensure correct offset calculations.
+ value_size: usize,
Review Comment:
This is the key change -- `value_length` is stored as a `usize` rather than
`i32` and then all pointer arithmetic is done in terms of `usize` rather than
`i32`
That is easier to verify / check for overflow
--
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]
