stevenzwu commented on code in PR #16025: URL: https://github.com/apache/iceberg/pull/16025#discussion_r4039611163
########## format/spec.md: ########## Review Comment: > Each manifest uses a single partition spec We need to fix this line here that is only correct for v1-v3 ########## format/spec.md: ########## @@ -1387,6 +1534,20 @@ At most one deletion vector is allowed per data file in a snapshot. If a DV is w [puffin-spec]: https://iceberg.apache.org/puffin-spec/ +#### Manifest Deletion Vectors + +A manifest deletion vector marks entries in a leaf manifest as not live by encoding their positions in a bitmap. A set bit at position P indicates that the entry at position P in the referenced leaf manifest is not live. + +Manifest deletion vectors are encoded using the [Mumbling bitmap spec][mumbling-spec] and stored inline on the root manifest entry that references the leaf manifest. The snapshot in which the vector last changed is recorded in `tracking.dv_snapshot_id`; the three bitmaps are: + +* `manifest_info.dv`: every position not live as of that snapshot. `manifest_info.dv_cardinality` is its cardinality. +* `tracking.deleted_positions`: the positions deleted in that snapshot. +* `tracking.replaced_positions`: the positions replaced in that snapshot. + +`deleted_positions` and `replaced_positions` are disjoint. Review Comment: Earlier (tracked file requirements), those same fields are only constrained by **may only be set**. Optional-field tables also mark all three as optional. I am wondering if we need to tighten up the wording here. - Writers MUST emit `deleted_positions` / `replaced_positions` whenever `manifest_info.dv` changes - If present, they MUST be **exactly** the positions newly deleted vs replaced **in this snapshot** - Readers MUST treat `manifest_info.dv` as authoritative for liveness and MAY use `deleted_positions` / `replaced_positions` for change detection. - In the next snapshot, writes MAY nullify the `deleted_positions` / `replaced_positions` and `dv_snapshot_id`. Without that, a writer that only stores the cumulative bitmap (and leaves `dv_snapshot_id` / the deltas null) is neither clearly conformant nor clearly wrong. -- 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]
