amogh-jahagirdar commented on code in PR #18526: URL: https://github.com/apache/iceberg/pull/18526#discussion_r4459712014
########## format/spec.md: ########## @@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0. +##### Snapshot Timestamp Inheritance + +In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage). + +The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows: Review Comment: Do we need the "so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. " statement? The rules below cover what to to do on write/read? ########## format/spec.md: ########## @@ -1053,7 +1059,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table #### Inheritance -Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. +Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4, `snapshot_timestamp` is also inherited when `null`; see [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance). Review Comment: v4 and later (or v4+ honestly, we say "v4 and later" a lot)... ########## format/spec.md: ########## @@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0. +##### Snapshot Timestamp Inheritance + +In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage). + +The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows: + +* An ADDED entry must have a `null` `snapshot_timestamp`. +* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`. +* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted). + +When reading, a `null` `snapshot_timestamp` is inherited differently depending on the entry location: Review Comment: I think we can drop the "inherited differently depending on.." and just have a combined statement (similar to what we do for other inheritance areas): ` When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest. ` -- 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]
