uros-b commented on code in PR #17783: URL: https://github.com/apache/iceberg/pull/17783#discussion_r3839615102
########## format/spec.md: ########## @@ -1416,6 +1417,8 @@ The rows in the delete file must be sorted by `file_path` then `pos` to optimize Equality delete files identify deleted rows in a collection of data files by one or more column values, and may optionally contain additional columns of the deleted row. +Equality deletes are not an allowed entry type in v4 metadata, so new equality deletes must not be written to v4 tables. Equality deletes carried over from v2 or v3 remain in the delete manifests that already track them and must still be applied when reading. Converting them to [deletion vectors](#deletion-vectors) is a separate, optional maintenance action. Review Comment: Minor note regarding phrasing inconsistency: "Equality deletes are not an allowed entry type in v4 metadata": The phrase appears in the new Equality Delete Files paragraph lead sentence (~line 1417) and again as the top-level bullet in Appendix E "Row-level delete changes:". Neither uses RFC-2119 language and both diverge from the precedents already in the same file: - Delete Formats (also added in this PR): "Equality delete files must not be added to v4 tables" — correct form, mirrors v3 exactly. - Appendix E v3: "Writers are not allowed to add new position delete files to v3 tables" — the positional precedent. - Appendix E v4 sub-bullet (also in this PR): "Writers must not write new equality deletes to v4 tables" — immediately below the problematic lead bullet. - Beyond style, "not an allowed entry type in v4 metadata" creates a latent read-confusion risk: the manifest schema still encodes content=2: EQUALITY_DELETES as a valid field value (needed to read legacy manifests), so saying the type is "not allowed" could mislead an implementer into thinking v4 readers must reject content=2 entries. The intent (write prohibition, not read prohibition) is clear from surrounding sentences but the lead wording undermines it. Recommended fix: replace both occurrences with wording that matches the Delete Formats sentence already in this PR, e.g. "Equality delete files must not be added to v4 tables, but existing equality delete files are valid", or align with the sub-bullet directly below: "Writers must not write new equality deletes to v4 tables." -- 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]
