[GitHub] [arrow] jhorstmann commented on a change in pull request #8645: ARROW-10561: [Rust] Small simplification of `write` and `write_bytes`

[GitBox <gi...@apache.org>]

jhorstmann commented on a change in pull request #8645:
URL: https://github.com/apache/arrow/pull/8645#discussion_r522001562



##########
File path: rust/arrow/src/array/builder.rs
##########
@@ -397,18 +396,9 @@ impl<T: ArrowPrimitiveType> BufferBuilder<T> {
     /// Writes a byte slice to the underlying buffer and updates the `len`, i.e. the
     /// number array elements in the builder.  Also, converts the `io::Result`
     /// required by the `Write` trait to the Arrow `Result` type.
-    fn write_bytes(&mut self, bytes: &[u8], len_added: usize) -> Result<()> {
-        let write_result = self.buffer.write(bytes);
-        // `io::Result` has many options one of which we use, so pattern matching is
-        // overkill here
-        if write_result.is_err() {
-            Err(ArrowError::MemoryError(
-                "Could not write to Buffer, not big enough".to_string(),
-            ))
-        } else {
-            self.len += len_added;
-            Ok(())
-        }
+    fn write_bytes(&mut self, bytes: &[u8], len_added: usize) {

Review comment:
       @jorgecarleitao My guess would be that the issue is related to this `len_added` parameter. In the filter kernel this was used for additional padding, most other users probably interpreted this as the length of the bytes array. I would suggest removing this parameter, since you already implemented a workaround in the filter kernel.




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