alamb commented on code in PR #1700:
URL: https://github.com/apache/arrow-rs/pull/1700#discussion_r873651342
##########
parquet/src/arrow/array_reader.rs:
##########
@@ -1564,12 +1541,74 @@ mod tests {
.collect::<Vec<bool>>()
);
assert_eq!(
- Some(vec![0, 1, 1, 1, 1].as_slice()),
+ Some(vec![0, 1, 2, 3, 1].as_slice()),
struct_array_reader.get_def_levels()
);
assert_eq!(
Some(vec![0, 1, 1, 1, 1].as_slice()),
struct_array_reader.get_rep_levels()
);
}
+
+ #[test]
+ fn test_struct_array_reader_list() {
+ use arrow::datatypes::Int32Type;
+ // [
+ // {foo: [1, 2, null],
+ // {foo: []},
+ // {foo: null},
+ // null,
+ // ]
+
+ let expected_l =
+ Arc::new(ListArray::from_iter_primitive::<Int32Type, _, _>(vec![
+ Some(vec![Some(1), Some(2), None]),
+ Some(vec![]),
+ None,
+ None,
+ ]));
+
+ let nulls = Buffer::from([0b00000111]);
Review Comment:
doesn't this set the first three elements in the struct array to `NULL`?
That doesn't seem consistent with the structure created in the comments above
##########
parquet/src/arrow/array_reader.rs:
##########
@@ -742,80 +737,59 @@ impl ArrayReader for StructArrayReader {
.collect::<Vec<ArrayData>>(),
);
- if self.struct_def_level != 0 {
+ if self.nullable {
// calculate struct def level data
- let buffer_size = children_array_len * size_of::<i16>();
- let mut def_level_data_buffer = MutableBuffer::new(buffer_size);
- def_level_data_buffer.resize(buffer_size, 0);
- // Safety: the buffer is always treated as `u16` in the code below
- let def_level_data = unsafe {
def_level_data_buffer.typed_data_mut() };
Review Comment:
+1 for removing some `unsafe`
##########
parquet/src/arrow/array_reader/map_array.rs:
##########
@@ -154,15 +150,15 @@ impl ArrayReader for MapArrayReader {
}
fn get_def_levels(&self) -> Option<&[i16]> {
- self.def_level_buffer
- .as_ref()
- .map(|buf| unsafe { buf.typed_data() })
+ // Children definition levels should describe the same parent
structure,
+ // so return key_reader only
+ self.key_reader.get_def_levels()
Review Comment:
I am a big fan of less `unsafe` 👍
--
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]