This is the partition field name, not a schema field name. The schema field must be looked up by source_id. This method needs both the original schema and the fresh schema. The original schema is used to get field names and then the fresh schema is used to look up the new source ID.

Great catch! 👍🏻

@Fokko, looks like this hasn't been fixed yet, so I'm reopening the thread.

Sorry, that slipped through somehow

Is this pre-order? In Java we called it CustomOrder because you can choose when to visit children by accessing the callable.

It is pre-order traversal since we start at the root and then move to the leaves. In order is a bit less intuitive since it is not a binary tree. You could also do a reverse in-order, but not sure if we need that. We can also call it CustomOrder if you have a strong preference, but I think pre-order is the most logical way of using this visitor.

Shouldn't this re-map the identifier field IDs since it is returning a new schema?

Yes, we do that in the function itself:

def assign_fresh_schema_ids(schema: Schema) -> Schema:
    """Traverses the schema, and sets new IDs"""
    schema_struct = pre_order_visit(schema.as_struct(), _SetFreshIDs())

    fresh_identifier_field_ids = []
    new_schema = Schema(*schema_struct.fields)
    for field_id in schema.identifier_field_ids:
        original_field_name = schema.find_column_name(field_id)
        if original_field_name is None:
            raise ValueError(f"Could not find field: {field_id}")
        fresh_field = new_schema.find_field(original_field_name)
        if fresh_field is None:
            raise ValueError(f"Could not lookup field in new schema: {original_field_name}")
        fresh_identifier_field_ids.append(fresh_field.field_id)

    return new_schema.copy(update={"identifier_field_ids": fresh_identifier_field_ids})

This is because we first want to know all the IDs

Ah, I've refactored this because we need to build a map anyway 👍🏻

This is going to visit children before visiting the next field. If you're trying to match the behavior of assignment in Java, you'd need to increment the counter for each field and then visit children.

Missed that one, thanks! Just updated the code and tests

Do the "fresh" methods always reset schema_id, spec_id, and order_id?

Only when you create TableMetadata out of it (when creating a new table). And it resets if it isn't 1.