gaborkaszab commented on code in PR #18791:
URL: https://github.com/apache/iceberg/pull/18791#discussion_r4477840952


##########
format/spec.md:
##########
@@ -855,7 +855,7 @@ A manifest file must store metadata as properties in the 
file's key-value metada
 
 #### Content file uniqueness
 
-Within a snapshot, each content file must be referenced by at most one live 
manifest entry across all manifests; otherwise, the snapshot has undefined 
behavior. Writers should not produce multiple manifest entries for the same 
content file in a snapshot (for example, both ADDED and DELETED entries for the 
same file). Writers are not required to validate uniqueness at commit time.
+Within a snapshot, each content file must be referenced by at most one live 
manifest entry across all manifests; otherwise, the snapshot has undefined 
behavior. Writers should not produce multiple manifest entries for the same 
content file in a snapshot (for example, both ADDED and DELETED entries for the 
same file), except for the REPLACED and MODIFIED pair produced when a file is 
modified. Writers are not required to validate uniqueness at commit time.

Review Comment:
   Thanks for the suggestion, @amogh-jahagirdar !
   
   Just trying to think it through from all angles before doing the change:
   With saying what you suggest aren't we in the situation that some status 
combinations aren't covered by the wording. E.g. This uniqueness paragraph says 
`DELETED` is not allowed together with live entries, but doesn't say anything 
about `DELETED` + `REPLACED` combination. Both are non-live so none of the 
sentences protect against that. Rules for `REPLACED` might be covered somewhere 
else in the spec, but the uniqueness section isn't self contained then.
   
   With saying what I suggested, that it's not allowed to have multiple entries 
for the same file except for the `REPLACED` + `MODIFIED` combo we kind of 
covered all the combinations, and maybe safe for potential future status 
changes too.
   
   WDYT @amogh-jahagirdar @RussellSpitzer ?



-- 
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]

Reply via email to