[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8853: ARROW-10827: [Rust] Move concat from builders to a compute kernel and make it faster (2-6x)

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

jorgecarleitao commented on a change in pull request #8853:
URL: https://github.com/apache/arrow/pull/8853#discussion_r542189448



##########
File path: rust/arrow/src/array/array_binary.rs
##########
@@ -373,6 +376,45 @@ impl From<Vec<Vec<u8>>> for FixedSizeBinaryArray {
     }
 }
 
+impl From<Vec<Option<Vec<u8>>>> for FixedSizeBinaryArray {
+    fn from(data: Vec<Option<Vec<u8>>>) -> Self {
+        let len = data.len();
+        assert!(len > 0);
+        // try to estimate the size. This may not be possible no entry is valid => panic

Review comment:
       Note that in general we should avoid using these because they require two allocations (rust's `Vec` and arrow buffers). This function is mostly useful for testing.
   
   I would be ok with replacing them by the `FromIter` constructor, which is more performance, more general, and has the same ergonomics (`from(vec![].into_iter())` instead of `from(vec![...])` for a vector). This way we do not need to worry about these.
   
   The challenge with fixed sized items is that they require knowledge of the size. This would be nicely solved by accepting `Option<[T; T: usize]>`, but Rust's support for constant generics is slim atm.




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