jorgecarleitao commented on a change in pull request #9215:
URL: https://github.com/apache/arrow/pull/9215#discussion_r563153376
##########
File path: rust/arrow/src/array/array_string.rs
##########
@@ -50,49 +50,65 @@ pub struct GenericStringArray<OffsetSize:
StringOffsetSizeTrait> {
}
impl<OffsetSize: StringOffsetSizeTrait> GenericStringArray<OffsetSize> {
- /// Returns the offset for the element at index `i`.
- ///
- /// Note this doesn't do any bound checking, for performance reason.
- #[inline]
- pub fn value_offset(&self, i: usize) -> OffsetSize {
- self.value_offset_at(self.data.offset() + i)
- }
-
/// Returns the length for the element at index `i`.
- ///
- /// Note this doesn't do any bound checking, for performance reason.
#[inline]
- pub fn value_length(&self, mut i: usize) -> OffsetSize {
- i += self.data.offset();
- self.value_offset_at(i + 1) - self.value_offset_at(i)
+ pub fn value_length(&self, i: usize) -> OffsetSize {
+ let offsets = self.value_offsets();
+ offsets[i + 1] - offsets[i]
}
- /// Returns a clone of the value offset buffer
- pub fn value_offsets(&self) -> Buffer {
- self.data.buffers()[0].clone()
+ /// Returns the offset values in the offsets buffer
+ #[inline]
+ pub fn value_offsets(&self) -> &[OffsetSize] {
Review comment:
what do you think of naming it simply `offsets`? I can't see the value
of `value_` on the name (same for the other types). Since we are touching the
API in backward incompatible ways... xD
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
