Jefffrey commented on code in PR #7419:
URL: https://github.com/apache/arrow-rs/pull/7419#discussion_r2679649929
##########
arrow-row/src/lib.rs:
##########
@@ -2539,4 +2553,17 @@ mod tests {
let rows = converter.convert_columns(&[Arc::new(a) as _]).unwrap();
assert_eq!(rows.row(0).cmp(&rows.row(1)), Ordering::Less);
}
+
+ #[test]
+ fn test_empty_struct() {
+ let s = Arc::new(StructArray::new_empty_fields(5, None)) as ArrayRef;
+
+ let sort_fields = vec![SortField::new(s.data_type().clone())];
+ let converter = RowConverter::new(sort_fields).unwrap();
+ let r = converter.convert_columns(&[Arc::clone(&s)]).unwrap();
+
+ let back = converter.convert_rows(&r).unwrap();
+ assert_eq!(back.len(), 1);
+ assert_eq!(&back[0], &s);
+ }
Review Comment:
We can do this as a followup; this PR is only for structarrays with no child
fields (the structarray itself isn't empty)
I don't think we can do that for other nested types 🤔
##########
arrow-row/src/lib.rs:
##########
@@ -1330,9 +1335,18 @@ fn encode_column(
.skip(1)
.enumerate()
.for_each(|(idx, offset)| {
- let (row, sentinel) = match array.is_valid(idx) {
- true => (rows.row(idx), 0x01),
- false => (*null, null_sentinel),
+ let (row, sentinel) = if array.is_valid(idx) {
+ let row = if rows.num_rows() == 0 {
Review Comment:
Took the liberty of refactoring this to pull the check outside the loop
(leads to a bit of code duplication but shouldn't impact runtime now)
> I think we could check if the input array's size was greater than zero and
just ignore the offsets if not
I don't think we can ignore offsets like that since we'd still need to
consider nulls 🤔
--
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]
