Hi, @AndreMouche Thanks for your contribution, I have some concerns for this pr:

for performance: remove unnecessary match operations that could have a negative impact on performance.

Is there any performance number to comprare these two methods?

Also please note that ArrowSchemaVisitor is a framework for processing ArrowSchema in visitor pattern, not only designed for iceberg schema converter. We will use it later when processing parquet files.

Is there any performance number to comprare these two methods?

Comparison of bench test results between current PR and master branches:
current branch:

➜  iceberg-rust git:(convert_arrow_filed_to_iceberg_field) ✗ cargo bench --package iceberg --lib -- arrow::schema::tests::bench_complex_arrow_schema_to_iceberg --exact --show-output 
    Finished `bench` profile [optimized] target(s) in 0.67s
     Running unittests src/lib.rs (target/release/deps/iceberg-ba39310710f5b54e)

running 1 test
test arrow::schema::tests::bench_complex_arrow_schema_to_iceberg ... bench:  29,431,166.70 ns/iter (+/- 1,098,685.87)

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 1 measured; 456 filtered out; finished in 9.02s

➜  iceberg-rust git:(convert_arrow_filed_to_iceberg_field) ✗ cargo bench --package iceberg --lib -- arrow::schema::tests::bench_complex_arrow_schema_to_iceberg --exact --show-output 
    Finished `bench` profile [optimized] target(s) in 0.56s
     Running unittests src/lib.rs (target/release/deps/iceberg-ba39310710f5b54e)

running 1 test
test arrow::schema::tests::bench_complex_arrow_schema_to_iceberg ... bench:  29,581,220.90 ns/iter (+/- 775,051.14)

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 1 measured; 456 filtered out; finished in 8.94s

master @ #567

➜  iceberg-rust git:(bench_mark_arrow_schema) cargo bench --package iceberg --lib -- arrow::schema::tests::bench_complex_arrow_schema_to_iceberg --exact --show-output 
    Finished `bench` profile [optimized] target(s) in 0.32s
     Running unittests src/lib.rs (target/release/deps/iceberg-ba39310710f5b54e)

running 1 test
test arrow::schema::tests::bench_complex_arrow_schema_to_iceberg ... bench:  30,535,066.60 ns/iter (+/- 640,812.07)

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 1 measured; 456 filtered out; finished in 9.26s

➜  iceberg-rust git:(bench_mark_arrow_schema) cargo bench --package iceberg --lib -- arrow::schema::tests::bench_complex_arrow_schema_to_iceberg --exact --show-output 
    Finished `bench` profile [optimized] target(s) in 0.32s
     Running unittests src/lib.rs (target/release/deps/iceberg-ba39310710f5b54e)

running 1 test
test arrow::schema::tests::bench_complex_arrow_schema_to_iceberg ... bench:  30,707,408.30 ns/iter (+/- 1,031,892.12)

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 1 measured; 456 filtered out; finished in 9.30s

Also please note that ArrowSchemaVisitor is a framework for processing ArrowSchema in visitor pattern, not only designed for iceberg schema converter. We will use it later when processing parquet files.****

make sense to me.

However, I still have few question about it.

Now it seems that we could use ArrowSchemaConverter in visitor pattern, could we make visit_type, visit_list as the as the function to ArrowSchemaConverter ?
ArrowSchemaConverter is designed as a tool to convert arrow_chema only, or a tool can convert shema and data types?

Meanwhile, about the interface ArrowSchemaVisitor

For example, we already know list should be a struct List, why we do not take the element_field of the list as the arguments directly?
Meanwhile, since ArrowSchemaVisitor is a general interface, could you please make the definitions of each interface function be made clearer? which will be very helpful for new newcomers like me, thanks.

friendly ping @Xuanwo @liurenjie1024

Hi, @AndreMouche Sorry for late reply.

It seems that the benchmark result shows that there is no critial performance change?

Now it seems that we could use ArrowSchemaConverter in visitor pattern, could we make visit_type, visit_list as the as the function to ArrowSchemaConverter ?

Sory I don't get your point. visit_type/visit_list are functions of SchemaVisitor, how could we make it part of ArrowSchemaConverter.

For example, we already know list should be a struct List, why we do not take the element_field of the list as the arguments directly?

I think this is a good suggestion. However, there are several kinds of lists in arrow, for example List, LargeList, FixedSizeList, so I think current design is a tradeoff so that we don't need to deal with all variants.

Meanwhile, since ArrowSchemaVisitor is a general interface, could you please make the definitions of each interface function be made clearer? which will be very helpful for new newcomers like me, thanks.

I totally agree that we should add some doc for new contributors.