I'm hesitating to add an iceberg_type here.

I also don't think we need an arrow_type, arrow array has data type associated with it: https://docs.rs/arrow/latest/arrow/array/trait.Array.html#tymethod.data_type

We need have clear documentation about what kind of visitor it is. Is it an post order visitor?

In Iceberg, everything is post-order, except assigning field-IDs. I think in this case post-order makes sense, since you would first convert the fields of the struct, and then move up to nested-structs, lists, maps, structs, all the way up to the schema struct.

The implementation of this method is somehow weird to me, since it's actually implementing the visiting scheduler logic, a typical post order visitor definition should look like this:

fn r#sturct(&self, array: &StructArray, children: Vec<Vec<Self::>>) -> Result<Vec<Self::T>>;

And we should move this logic into a standalone function like this:

fn visit(array: &Array, visitor: V) -> Result<V::T> {
 ...
}

Looping back @liurenjie1024 question here:

We need have clear documentation about what kind of visitor it is.

First of all, thanks for converting this into a visitor, I think that's the right approach here. One question regarding the lookup. It looks like we're expecting the Arrow data to be in the same position of the Iceberg schema. Instead, what would be more Iceberg-esque, is performing the lookup by field-ID.

For example:

schema = Schema(
    NestedField(1, "city", StringType(), required=False),
    NestedField(2, "lat", DoubleType(), required=False),
    NestedField(3, "long", DoubleType(), required=False),
)

table {
  1: city: optional string
  2: lat: optional double
  3: long: optional double
}

table = catalog.create_table("default.locations", schema)

with table.update_schema() as update:
    update.move_after("lat", "long")


table {
  1: city: optional string
  3: lat: optional double
  2: long: optional double
}

Now the fields are switched. Data that's already written needs to be projected, and new data can be read as is. Both of the files will be read by this visitor without raising an error, but for the old files, the data the fields will be switched.

This way, we could also do smart things, like fill in nullable fields:

        let struct_array = StructArray::new_null();
        let iceberg_struct_type = StructType::new(vec![Arc::new(NestedField::optional(
            0,
            "bool_field",
            Type::Primitive(PrimitiveType::Boolean),
        ))]);
        let result = arrow_struct_to_literal(&struct_array, iceberg_struct_type).unwrap();
        assert_eq!(result, vec![None; 3]);

And in a step after that, #737. You can find the Python equivalent here.

Use field-ID require user store the file id metadata for date type in struct type. I'm not sure whether is this always set up. How about providing two visit function? One would assume the schema of arrow will be same as iceberg schema, it will return error if convert fail. And the other one will look up the column using the field id.