syun64 commented on code in PR #305:
URL: https://github.com/apache/iceberg-python/pull/305#discussion_r1470494285
##########
pyiceberg/schema.py:
##########
@@ -1221,50 +1221,57 @@ def assign_fresh_schema_ids(schema_or_type:
Union[Schema, IcebergType], next_id:
class _SetFreshIDs(PreOrderSchemaVisitor[IcebergType]):
"""Traverses the schema and assigns monotonically increasing ids."""
- reserved_ids: Dict[int, int]
+ old_id_to_new_id: Dict[int, int]
Review Comment:
I tried this out, and I think we might have to leave this logic in for this
iteration.
In PyIceberg, a user can create a table using IcebergSchema, instead of a
PyArrow schema. This means that the user can choose to include
identifier_field_ids in the IcebergSchema that they define, and the current
create_table or assign_fresh_ids function calls don’t have the logic to handle
this outside of _SetFreshIDs
I think it might make sense to consolidate the way we update the
identifier_field_ids on create_table, update_schema, replace_table and other
similar operations in a future PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
