ericwburden commented on a change in pull request #9624:
URL: https://github.com/apache/arrow/pull/9624#discussion_r586473814



##########
File path: rust/arrow/src/array/array_binary.rs
##########
@@ -89,7 +89,7 @@ impl<OffsetSize: BinaryOffsetSizeTrait> 
GenericBinaryArray<OffsetSize> {
         // pointer alignment & location is ensured by RawPtrBox
         // buffer bounds/offset is ensured by the value_offset invariants
         std::slice::from_raw_parts(
-            self.value_data.as_ptr().offset(start.to_isize()),
+            self.value_data.as_ptr().offset(start.to_isize().unwrap()),

Review comment:
       Here's my rationale:
   
   > `start` is an &OffsetSize, which is a generic type that implements the 
   > OffsetSizeTrait. Currently, only i32 and i64 implement OffsetSizeTrait,
   > both of which should cleanly cast to isize on an architecture that supports
   > 32/64-bit offsets
   
   I *think* that is true. Can anyone think of a situation in which that would 
not hold? (For reference, I'm primarily an R programmer, so questions about 
memory address size are a bit arcane for me).




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to