soumya-ghosh commented on code in PR #956:
URL: https://github.com/apache/iceberg-python/pull/956#discussion_r1687693813
##########
pyiceberg/table/__init__.py:
##########
@@ -1178,10 +1178,15 @@ def update_table_metadata(
"""
context = _TableMetadataUpdateContext()
new_metadata = base_metadata
+ new_metadata = new_metadata.model_copy(update={"last_updated_ms": 0})
for update in updates:
new_metadata = _apply_table_update(update, new_metadata, context)
Review Comment:
I agree with @HonahX. @kevinjqliu, what you are suggesting did cross my mind
but I thought it would be better to avoid setting `last-updated-ms` at multiple
places.
I was checking the how this is handled in Iceberg, here a metadata instance
is created with `lastUpdatedMillis` set to null
https://github.com/apache/iceberg/blob/3c46178855ecec864e1ea9ff67392c0c96d49e0d/core/src/main/java/org/apache/iceberg/TableMetadata.java#L937
Then in the build method, `lastUpdatedMillis` is updated to current time if
not already set. It also has several checks to determine if there are any
updated in metadata
https://github.com/apache/iceberg/blob/3c46178855ecec864e1ea9ff67392c0c96d49e0d/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1431-L1438
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
