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

Code before:
  out->children.resize(group.field_count());
  SchemaField* child_field = &out->children[0];

  ctx->LinkParent(out, parent);
  ctx->LinkParent(child_field, out);

  const Node& list_node = *group.field(0);

  if (!list_node.is_repeated()) {
    return Status::Invalid(
        "Non-repeated nodes in a LIST-annotated group are not supported.");
  }

  int16_t repeated_ancestor_def_level = current_levels.IncrementRepeated();
  if (list_node.is_group()) {
    // Resolve 3-level encoding
    //
    // required/optional group name=whatever {
    //   repeated group name=list {
    //     required/optional TYPE item;
    //   }
    // }
    //
    // 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
    //

Review comment (bug):
Thanks for the careful explanation on my last questions, after rereading I think I mostly agree that this is a bug that needs to be fixed.  I think I missed the second (LIST) annotation.  Which even though this check corresponds to the java, code, it seems an important factor here is that the list_group is in fact a (LIST and maybe even possibly a map), not that the inner element is repeated? 

So I think the logic might make more sense as (pseudocode):

```
if (list_group.field_count() > 1) {
   ...
} else if (HasListElementName(list_group, group)) {
   if (IsMap(list_group)) {
           RETURN_NOT_OK(
            ListToMapField(*list_group, current_levels, ctx, out, child_field));
      
   } else if IsList(list_group)) {
      RETURN_NOT_OK(
            ListToSchemaField(*list_group, current_levels, ctx, out, child_field));
  } else {
      RETURN_NOT_OK(GroupToStruct(list_group, current_levels, ctx, out, child_field));
  }
} else {
  ...
}
``` 
Does this formulation work?

Code after:

  current_levels.Increment(group);

  out->children.resize(group.field_count());
  SchemaField* child_field = &out->children[0];

  ctx->LinkParent(out, parent);
  ctx->LinkParent(child_field, out);

  const Node& list_node = *group.field(0);

  if (!list_node.is_repeated()) {
    return Status::Invalid(
        "Non-repeated nodes in a LIST-annotated group are not supported.");
  }

  int16_t repeated_ancestor_def_level = current_levels.IncrementRepeated();
  if (list_node.is_group()) {
    const auto& list_group = static_cast<const GroupNode&>(list_node);
    if (list_group.field_count() > 1) {
      // 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 {