nevi-me commented on a change in pull request #8792:
URL: https://github.com/apache/arrow/pull/8792#discussion_r532205471
##########
File path: rust/parquet/src/arrow/arrow_writer.rs
##########
@@ -608,16 +631,31 @@ fn get_levels(
/// In the case where the array in question is a child of either a list or
struct, the levels
/// are incremented in accordance with the `level` parameter.
/// Parent levels are either 0 or 1, and are used to higher (correct
terminology?) leaves as null
+///
+/// TODO: (a comment to remove, note to help me reduce the mental bookkeeping)
+/// We want an array's levels to be additive here, i.e. if we have an array
that
+/// comes from <batch<primitive>>, we should consume &[0; array.len()], so that
+/// we add values to it, instead of subtract values
+///
+/// An alternaitve is to pass the max level, and use it to compute whether we
+/// should increment (though this is likely tricker)
fn get_primitive_def_levels(
array: &arrow_array::ArrayRef,
+ field: &Field,
parent_def_levels: &[i16],
) -> Vec<i16> {
let mut array_index = 0;
let max_def_level = parent_def_levels.iter().max().unwrap();
let mut primitive_def_levels = vec![];
parent_def_levels.iter().for_each(|def_level| {
- if def_level < max_def_level {
+ // TODO: if field is non-nullable, can its parent be nullable? Ideally
shouldn't
Review comment:
It still isn't, the current tests don't hit this line, so I'll only be
able to confirm it once I have `<list<struct<...>>>` or some other combination
like that.
----------------------------------------------------------------
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:
us...@infra.apache.org
