[GitHub] [arrow-rs] alamb commented on a diff in pull request #3182: Add support for FixedSizeBinary in Row format

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

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


##########
arrow/src/row/mod.rs:
##########
@@ -809,6 +809,10 @@ fn new_empty_rows(
                 .iter()
                 .zip(lengths.iter_mut())
                 .for_each(|(slice, length)| *length += variable::encoded_len(slice)),
+            DataType::FixedSizeBinary(len) => {

Review Comment:
   I double checked and there is no such thing as `FixedSizeLargeBinary` https://github.com/apache/arrow-rs/blob/6f41b95de4a0ac33319b9e96e179b47fcd71fdbf/arrow-schema/src/datatype.rs#L155-L157 👍 



##########
arrow/src/row/fixed.rs:
##########
@@ -201,6 +201,29 @@ pub fn encode<T: FixedLengthEncoding, I: IntoIterator<Item = Option<T>>>(
     }
 }
 
+pub fn encode_fixed_size_binary(
+    out: &mut Rows,
+    array: &FixedSizeBinaryArray,
+    opts: SortOptions,
+) {
+    let len = array.value_length() as usize;
+    for (offset, maybe_val) in out.offsets.iter_mut().skip(1).zip(array.iter()) {
+        let end_offset = *offset + len + 1;
+        if let Some(val) = maybe_val {
+            let to_write = &mut out.buffer[*offset..end_offset];
+            to_write[0] = 1;

Review Comment:
   Maybe it is worth noting that this value is chosen to be neither 0 or 0xff (the null sentinels)?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org