Thanks @rdblue for the comments. I wanted to get a sense from you for the overall approach here, specifically around the mapping , where the fieldname to id map contains non canonical field names and testing strategy. Do you think the current approach is sound enough?

Hi @rdblue , please have a look when u get the time. I've addressed your remaining comments. I need your inputs specifically on some of the TODOs I've added to the code.

Sorry for not getting back to this very quickly. I'm planning to review it this week.

@rdsr, thanks for your patience. This looks like a great start.

I made a few specific comments. In addition, I think we may want to apply the schema mapping differently. Instead of building an Avro schema with IDs, why not add an optional mapping to the conversion from Avro to Iceberg schema? Then that conversion could drop fields that don't have IDs, either embedded in the schema or from a mapping. I think that would fit a bit more cleanly because it would not be creating an intermediate file schema. It would be producing the Iceberg equivalent of the Avro schema instead. What do you think?

I think the passing an optional mapping to the conversion from Avro to Iceberg schema makes sense. Let me investigate more on this.

Thanks for the comments!

@rdsr, I've been working on #41, and I think we may have some restructuring to the mapping, instead of a simple name to ID map. Supporting aliases with a simple map requires enumerating all of the possibilities, and it is difficult to work with. I think a multi-level approach, where each struct has a mapping is going to be better to work with. I'll clean up what I have and post a PR for you to look at.

Thanks @rdblue !

@rdblue , picking this up again and going through your changes in #41 .

I think the idea passing an optional mapping to avro to iceberg schema is workable, and I think u mean this api AvroSchemaUtil#buildAvroProjection.

I'm working on this this week and will try to post a patch soon

@rdblue I've updated the approach here.

I now pass an optional NameMapping to BuildAvroProjection
field ids are now determined from first the field-id property and then from NameMapping .
The logic of how a field is selected in each of the methods (field(), map(), array() etc) is also slightly changed.

The general logic now is

1. If the field does not have the field id (either as a property or in NameMapping) it is not selected
2. if it resulting child node is null it is not selected
3. if its resulting child node is different, a new field is created.

Thanks for the update, @rdsr! I'll have a look next week when I'm back.

I've updated the BuildAvroProjection to rename missing optional fields.
Also updated tests to have a complete round trip to data file and back.

Requesting a review @rdblue !

Merging this. Nice work, @rdsr! Thanks for taking care of this.