Repository: apache/arrow
File: cpp/src/parquet/arrow/schema.cc (C++)

Code before:
    //   }
    // }
    //
    // yields list<item: TYPE ?nullable> ?nullable
    //
    // We distinguish the special case that we have
    //
    // required/optional group name=whatever {
    //   repeated group name=array or $SOMETHING_tuple {
    //     required/optional TYPE item;
    //   }
    // }
    //
    // In this latter case, the inner type of the list should be a struct
    // rather than a primitive value
    //
    // yields list<item: struct<item: TYPE ?nullable> not null> ?nullable
    const auto& list_group = static_cast<const GroupNode&>(list_node);
    // Special case mentioned in the format spec:
    //   If the name is array or ends in _tuple, this should be a list of struct
    //   even for single child elements.
    if (list_group.field_count() == 1 && !HasStructListName(list_group)) {
      // List of primitive type
      RETURN_NOT_OK(
          NodeToSchemaField(*list_group.field(0), current_levels, ctx, out, child_field));
    } else {
      RETURN_NOT_OK(GroupToStruct(list_group, current_levels, ctx, out, child_field));
    }
  } else {
    // Two-level list encoding
    //
    // required/optional group LIST {
    //   repeated TYPE;
    // }
    const auto& primitive_node = static_cast<const PrimitiveNode&>(list_node);
    int column_index = ctx->schema->GetColumnIndex(primitive_node);
    ASSIGN_OR_RAISE(std::shared_ptr<ArrowType> type,
                    GetTypeForNode(column_index, primitive_node, ctx));
    auto item_field = ::arrow::field(list_node.name(), type, /*nullable=*/false,
                                     FieldIdMetadata(list_node.field_id()));
    RETURN_NOT_OK(
        PopulateLeaf(column_index, item_field, current_levels, ctx, out, child_field));
  }
  out->field = ::arrow::field(group.name(), ::arrow::list(child_field->field),
                              group.is_optional(), FieldIdMetadata(group.field_id()));
  out->level_info = current_levels;
  // At this point current levels contains the def level for this list,
  // we need to reset to the prior parent.
  out->level_info.repeated_ancestor_def_level = repeated_ancestor_def_level;
  return Status::OK();
}

Review comment (bug):
i'm not sure this is correct, or at least the comments above need to be updated to explain the logic further.

Specifially from the exapmles on logical type.md:

```
// List<OneTuple<String>> (nullable list, non-null elements)
optional group my_list (LIST) {
  repeated group array {
    required binary str (STRING);
  };
}
```

This seems to imply that despite how the file was written with Avro bindings there should in fact be an intermediate struct and not a llist<list<>>, its not clear to me if this is a bug in the spec, or a bug in Avro java writer implementation.



Code after:
      // The inner type of the list should be a struct when there are multiple fields
      // in the repeated group
      RETURN_NOT_OK(GroupToStruct(list_group, current_levels, ctx, out, child_field));
    } else if (list_group.field_count() == 1) {
      const auto& repeated_field = list_group.field(0);
      if (repeated_field->is_repeated()) {
        // Special case where the inner type might be a list with two-level encoding
        // like below:
        //
        // required/optional group name=SOMETHING (LIST) {
        //   repeated group array (LIST) {
        //     repeated TYPE item;
        //   }
        // }
        //
        // yields list<item: list<item: TYPE not null> not null> ?nullable
        if (!list_group.logical_type()->is_list()) {
          return Status::Invalid("Group with one repeated child must be LIST-annotated.");
        }
        // LIST-annotated group with three-level encoding cannot be repeated.
        if (repeated_field->is_group() &&
            !static_cast<const GroupNode&>(*repeated_field).field(0)->is_repeated()) {
          return Status::Invalid("LIST-annotated groups must not be repeated.");
        }
        RETURN_NOT_OK(
            NodeToSchemaField(*repeated_field, current_levels, ctx, out, child_field));
      } else if (HasListElementName(list_group, group)) {
        // We distinguish the special case that we have
        //
        // required/optional group name=SOMETHING {
        //   repeated group name=array or $SOMETHING_tuple {
        //     required/optional TYPE item;
        //   }
        // }
        //
        // The inner type of the list should be a struct rather than a primitive value
        //
        // yields list<item: struct<item: TYPE ?nullable> not null> ?nullable
        RETURN_NOT_OK(GroupToStruct(list_group, current_levels, ctx, out, child_field));
      } else {
        // Resolve 3-level encoding
        //
        // required/optional group name=whatever {
        //   repeated group name=list {
        //     required/optional TYPE item;
        //   }
        // }
        //
        // yields list<item: TYPE ?nullable> ?nullable
        RETURN_NOT_OK(
            NodeToSchemaField(*repeated_field, current_levels, ctx, out, child_field));