sungwy commented on code in PR #1304:
URL: https://github.com/apache/iceberg-python/pull/1304#discussion_r1855272561
##########
pyiceberg/table/metadata.py:
##########
@@ -517,12 +517,15 @@ def new_table_metadata(
location: str,
properties: Properties = EMPTY_DICT,
table_uuid: Optional[uuid.UUID] = None,
+ assign_fresh_ids: bool = True,
) -> TableMetadata:
from pyiceberg.table import TableProperties
- fresh_schema = assign_fresh_schema_ids(schema)
- fresh_partition_spec = assign_fresh_partition_spec_ids(partition_spec,
schema, fresh_schema)
- fresh_sort_order = assign_fresh_sort_order_ids(sort_order, schema,
fresh_schema)
+ if assign_fresh_ids:
+ fresh_schema = assign_fresh_schema_ids(schema)
+ partition_spec = assign_fresh_partition_spec_ids(partition_spec,
schema, fresh_schema)
+ sort_order = assign_fresh_sort_order_ids(sort_order, schema,
fresh_schema)
+ schema = fresh_schema
Review Comment:
Hi @kevinjqliu thank you for the review! Yes, I agree that the code path
would be simpler if we didn't expose `assign_fresh_ids` as a parameter for the
API. However, I think there were some concerns that were raised in not
surfacing that as an argument and having two code paths based strictly on the
input parameter. https://github.com/apache/iceberg-python/issues/1284
I will add this to the agenda for the PyIceberg Sync on Tuesday and see if
we that will help the community in reaching a consensus.
--
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]