RussellSpitzer commented on code in PR #17198: URL: https://github.com/apache/iceberg/pull/17198#discussion_r3582397968
########## format/spec.md: ########## @@ -700,6 +700,10 @@ The `sequence_number` field represents the data sequence number and must never c The `file_sequence_number` field represents the sequence number of the snapshot that added the file and must also remain unchanged upon assigning at commit. The file sequence number can't be used for pruning delete files as the data within the file may have an older data sequence number. The data and file sequence numbers are inherited only if the entry status is 1 (added). If the entry status is 0 (existing) or 2 (deleted), the entry must include both sequence numbers explicitly. +#### Content file uniqueness + +Within a snapshot, each live manifest entry must be uniquely defined by `file_path` across all manifest files. A snapshot with multiple live entries for the same `file_path` has undefined behavior. Writers are not required to validate uniqueness because doing so can be expensive at commit time. Review Comment: The previous language (that which we added to the bottom 4 years ago) tries to leverage "Live (ADDED, Existing)" to emphasize the logical nature of the existence. I'm open to any suggestions on new wording though if you think there is a better way. Honestly I'd love to not metntion 'file_path' at all, location is probably better but if we could say it without the field name that would be best. I just don't want to leave open a window to have "a.parquet - some stats" and "a.parquet - some other stats" which would be technically unique but violate the "same file" rule. ########## format/spec.md: ########## @@ -700,6 +700,10 @@ The `sequence_number` field represents the data sequence number and must never c The `file_sequence_number` field represents the sequence number of the snapshot that added the file and must also remain unchanged upon assigning at commit. The file sequence number can't be used for pruning delete files as the data within the file may have an older data sequence number. The data and file sequence numbers are inherited only if the entry status is 1 (added). If the entry status is 0 (existing) or 2 (deleted), the entry must include both sequence numbers explicitly. +#### Content file uniqueness + +Within a snapshot, each live manifest entry must be uniquely defined by `file_path` across all manifest files. A snapshot with multiple live entries for the same `file_path` has undefined behavior. Writers are not required to validate uniqueness because doing so can be expensive at commit time. Review Comment: The previous language (that which we added to the bottom 4 years ago) tries to leverage "Live (ADDED, Existing)" to emphasize the logical nature of the existence. I'm open to any suggestions on new wording though if you think there is a better way. Honestly I'd love to not mention 'file_path' at all, location is probably better but if we could say it without the field name that would be best. I just don't want to leave open a window to have "a.parquet - some stats" and "a.parquet - some other stats" which would be technically unique but violate the "same file" rule. -- 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]
