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


##########
arrow-data/src/byte_view.rs:
##########
@@ -32,6 +32,18 @@ pub struct ByteView {
 }
 
 impl ByteView {
+    /// Try to create a [`ByteView`] from the provided `v`
+    ///
+    /// If `v` instead contains the binary data inline, returns an `Err` 
containing it
+    #[inline]
+    pub fn try_new(v: &u128) -> Result<Self, &[u8]> {

Review Comment:
   I think what most confuses me about ByteView as a struct is that it doesn't 
represent in Rust anywhere the different layouts of the `u128`s in 
ByteViewArrays
   
   For example, if you look at the rust `ByteView` struct without consulting 
the arrow spec, you may come to the conclusion that the `u128`s in a 
ByteViewArray have this format, which is not the case and thus you need to
   1. Know to check "is length less than 12" and if so handle things specially 
(this API helps here by encapsulating that check for certain cases)
   2. Know how to construct a view from bytes (aka how much of the prefix to 
copy and where. 
   
   This API seems to improve things (though I think if we went this way we 
should expand its docstring to explain the difference between the two types of 
byte views)
   
   
   
   
   
   
   



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