alamb commented on code in PR #8252:
URL: https://github.com/apache/arrow-rs/pull/8252#discussion_r2326093230


##########
arrow-array/src/array/list_array.rs:
##########
@@ -37,7 +37,9 @@ use std::sync::Arc;
 /// [`LargeBinaryArray`]: crate::array::LargeBinaryArray
 /// [`StringArray`]: crate::array::StringArray
 /// [`LargeStringArray`]: crate::array::LargeStringArray
-pub trait OffsetSizeTrait: ArrowNativeType + std::ops::AddAssign + Integer {
+pub trait OffsetSizeTrait:
+    ArrowNativeType + std::ops::AddAssign + Integer + num::CheckedAdd

Review Comment:
   I think it is ok in general, but this might be a "breaking API change" 
potentially if any downstream crates have implemented the OffsetSizeTrait
   
   However, that seems unlikely so maybe it is ok



##########
arrow-array/src/builder/generic_bytes_builder.rs:
##########
@@ -142,9 +143,10 @@ impl<T: ByteArrayType> GenericByteBuilder<T> {
     /// Appends array values and null to this builder as is
     /// (this means that underlying null values are copied as is).
     #[inline]
-    pub fn append_array(&mut self, array: &GenericByteArray<T>) {
+    pub fn append_array(&mut self, array: &GenericByteArray<T>) -> Result<(), 
ArrowError> {

Review Comment:
   I believe this is also a public API change (and thus would have to wait for 
the next major release (scheduled for october)
   
   
https://docs.rs/arrow/latest/arrow/array/struct.GenericByteBuilder.html#method.append_array



-- 
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]

Reply via email to