stevenzwu commented on code in PR #16025: URL: https://github.com/apache/iceberg/pull/16025#discussion_r4234230292
########## format/spec.md: ########## @@ -144,8 +146,11 @@ Version 4 of the Iceberg spec adds support for relative locations in metadata, e * **Schema** -- Names and types of fields in a table. * **Partition spec** -- A definition of how partition values are derived from data fields. * **Snapshot** -- The state of a table at some point in time, including the set of all data files. -* **Manifest list** -- A file that lists manifest files; one per snapshot. -* **Manifest** -- A file that lists data or delete files; a subset of a snapshot. +* **Snapshot root file** -- The per-snapshot file that tracks a snapshot's manifests; a manifest list (v1-v3) or a root manifest (v4). Review Comment: nit: `root manifest (v4)` -> `root manifest (v4+)`, which would also align with the root manifest 2 lines below ########## format/spec.md: ########## @@ -476,7 +481,7 @@ A data file with only new rows for the table may omit the `_last_updated_sequenc On read, if `_last_updated_sequence_number` is `null` it is assigned the `sequence_number` of the data file's manifest entry. The data sequence number of a data file is documented in [Sequence Number Inheritance](#sequence-number-inheritance). -When `null`, a row's `_row_id` field is assigned to the `first_row_id` from its containing data file plus the row position in that data file (`_pos`). A data file's `first_row_id` field is assigned using inheritance and is documented in [First Row ID Inheritance](#first-row-id-inheritance). A manifest's `first_row_id` is assigned when writing the manifest list for a snapshot and is documented in [First Row ID Assignment](#first-row-id-assignment). A snapshot's `first-row-id` is set to the table's `next-row-id` and is documented in [Snapshot Row IDs](#snapshot-row-ids). +When `null`, a row's `_row_id` field is assigned to the `first_row_id` from its containing data file plus the row position in that data file (`_pos`). A data file's `first_row_id` field is assigned using inheritance and is documented in [First Row ID Inheritance](#first-row-id-inheritance). A manifest's `first_row_id` is assigned when writing the snapshot root file and is documented in [First Row ID Assignment](#first-row-id-assignment). In v4, a data file in the root manifest is assigned a `first_row_id` in the same way. A snapshot's `first-row-id` is set to the table's `next-row-id` and is documented in [Snapshot Row IDs](#snapshot-row-ids). Review Comment: > In v4, a data file in the root manifest is assigned a `first_row_id` in the same way. nit: A data file in the root manifest (v4+) is assigned a `first_row_id` in the same way. ########## format/spec.md: ########## @@ -1043,35 +1183,41 @@ Notes: #### First Row ID Assignment -The `first_row_id` for existing manifests must be preserved when writing a new manifest list. The value of `first_row_id` for delete manifests is always `null`. The `first_row_id` is only assigned for data manifests that do not have a `first_row_id`. Assignment must account for data files that will be assigned `first_row_id` values when the manifest is read. +The `first_row_id` for existing manifests must be preserved when writing a new snapshot root file. The value of `first_row_id` for delete manifests is always `null`. The `first_row_id` is only assigned for data manifests that do not have a `first_row_id`. Assignment must account for data files that will be assigned `first_row_id` values when the manifest is read. Data files in the root manifest (v4) must also have a `first_row_id`: existing values must be preserved and `first_row_id` is assigned when a data file is ADDED. + +The first file in the snapshot root file without a `first_row_id` is assigned a value that is greater than or equal to the `first_row_id` of the snapshot. Subsequent files without a `first_row_id` are assigned one based on the previous file to be assigned a `first_row_id`. Each assigned `first_row_id` must be greater than or equal to the last assigned `first_row_id` plus the row count of the last assigned file, where the row count is: -The first manifest without a `first_row_id` is assigned a value that is greater than or equal to the `first_row_id` of the snapshot. Subsequent manifests without a `first_row_id` are assigned one based on the previous manifest to be assigned a `first_row_id`. Each assigned `first_row_id` must increase by the row count of all files that will be assigned a `first_row_id` via inheritance in the last assigned manifest. That is, each `first_row_id` must be greater than or equal to the last assigned `first_row_id` plus the total record count of data files with a null `first_row_id` in the last assigned manifest. +* For a manifest, the total record count of data files with a null `first_row_id` in the manifest. +* For a data file, its `record_count`. A simple and valid approach is to estimate the number of rows in data files that will be assigned a `first_row_id` using the manifest's `added_rows_count` and `existing_rows_count`: `first_row_id = last_assigned.first_row_id + last_assigned.added_rows_count + last_assigned.existing_rows_count`. ### Scan Planning -Scans are planned by reading the manifest files for the current snapshot. Deleted entries in data and delete manifests (those marked with status "DELETED") are not used in a scan. +A reader plans a scan by producing live data files from the snapshot root file and any leaf manifests referenced by the root. Review Comment: nit: `any leaf manifests` -> `any live leaf manifests`. ########## format/spec.md: ########## @@ -977,18 +1117,18 @@ For other optional snapshot summary fields, see [Appendix F](#optional-snapshot- Data and delete files for a snapshot can be stored in more than one manifest. This enables: * Appends can add a new manifest to minimize the amount of data written, instead of adding new records by rewriting and appending to an existing manifest. (This is called a “fast append”.) -* Tables can use multiple partition specs. A table’s partition configuration can evolve if, for example, its data volume changes. Each manifest uses a single partition spec, and queries do not need to change because partition filters are derived from data predicates. +* Tables can use multiple partition specs. A table’s partition configuration can evolve if, for example, its data volume changes. Partition predicates for a partition spec are derived from data predicates and can be applied to filter files written using that spec. Prior to v4, a manifest stored files partitioned by single spec. Review Comment: > Prior to v4, a manifest stored files partitioned by single spec. The sentence feels incomplete without talking about v4+. Also since this is already covered in the Manifests section (copied below), maybe we should just drop it here to avoid duplicates. ``` v1-v3: All entries in a manifest use the same partition spec, because the manifest's partition struct schema is derived from that spec. v4: A manifest may store files written with different partition specs. ``` ########## format/spec.md: ########## @@ -1043,35 +1183,41 @@ Notes: #### First Row ID Assignment -The `first_row_id` for existing manifests must be preserved when writing a new manifest list. The value of `first_row_id` for delete manifests is always `null`. The `first_row_id` is only assigned for data manifests that do not have a `first_row_id`. Assignment must account for data files that will be assigned `first_row_id` values when the manifest is read. +The `first_row_id` for existing manifests must be preserved when writing a new snapshot root file. The value of `first_row_id` for delete manifests is always `null`. The `first_row_id` is only assigned for data manifests that do not have a `first_row_id`. Assignment must account for data files that will be assigned `first_row_id` values when the manifest is read. Data files in the root manifest (v4) must also have a `first_row_id`: existing values must be preserved and `first_row_id` is assigned when a data file is ADDED. + +The first file in the snapshot root file without a `first_row_id` is assigned a value that is greater than or equal to the `first_row_id` of the snapshot. Subsequent files without a `first_row_id` are assigned one based on the previous file to be assigned a `first_row_id`. Each assigned `first_row_id` must be greater than or equal to the last assigned `first_row_id` plus the row count of the last assigned file, where the row count is: -The first manifest without a `first_row_id` is assigned a value that is greater than or equal to the `first_row_id` of the snapshot. Subsequent manifests without a `first_row_id` are assigned one based on the previous manifest to be assigned a `first_row_id`. Each assigned `first_row_id` must increase by the row count of all files that will be assigned a `first_row_id` via inheritance in the last assigned manifest. That is, each `first_row_id` must be greater than or equal to the last assigned `first_row_id` plus the total record count of data files with a null `first_row_id` in the last assigned manifest. +* For a manifest, the total record count of data files with a null `first_row_id` in the manifest. Review Comment: nit: `total record count` -> `total row count`. Tracked file schema used "record count" as the number of entries in the leaf manifest file. ########## format/spec.md: ########## @@ -1367,7 +1522,7 @@ When removing a data file, writers must also remove any deletion vector that app Row-level delete files (both equality and position delete files) are valid Iceberg data files: files must use valid Iceberg formats, schemas, and column projection. It is recommended that these delete files are written using the table's default file format. -Row-level delete files and deletion vectors are tracked by manifests. A separate set of manifests is used for delete files and DVs, but the same manifest schema is used for both data and delete manifests. Deletion vectors are tracked individually by file location, offset, and length within the containing file. Deletion vector metadata must include the referenced data file. +Row-level delete files and deletion vectors are tracked by manifests. A separate set of manifests is used for delete files and DVs, but the same manifest schema is used for both data and delete manifests. Deletion vectors are tracked individually by file location, offset, and length within the containing file. Deletion vector metadata must include the referenced data file. A deletion vector may instead be colocated with its data file, and writers must colocate new deletion vectors in v4. Deletion vectors in delete manifests written before an upgrade to v4 remain valid. Review Comment: This paragraph mixes the v2/v3 representation, upgrade compatibility, and the v4 writer requirement. Could we separate them? The `may` refers to existing deletion vectors in pre-v4 delete manifests remaining valid, while v4 writers must record new deletion vectors in the `deletion_vector` struct on the data file’s tracked-file entry. > In v2 and v3, delete files and deletion vectors are tracked in delete manifests, using the same manifest schema as data manifests. A deletion vector in a delete manifest is identified by its containing file location, offset, and length, and its metadata must include the referenced data file. > Deletion vectors in delete manifests written before an upgrade to v4 remain valid. In v4, writers must record deletion vectors in the deletion_vector struct on the tracked-file entry for the referenced data file. The colocated entry may be stored in a leaf manifest or directly in the root manifest. ########## format/spec.md: ########## @@ -924,26 +1043,33 @@ Fields with stats tracked in `content_stats` change based on updates like schema A simple (and recommended) way for writers to adapt existing metadata for table changes is to read manifests with the implementation's current `content_stats` type and apply schema evolution rules, such as reading `int` as `long` for promoted fields. -#### Sequence Number Inheritance +#### 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. + +##### Sequence Number Inheritance Manifests track the sequence number when a data or delete file was added to the table. -When adding a new file, its data and file sequence numbers are set to `null` because the snapshot's sequence number is not assigned until the snapshot is successfully committed. When reading, sequence numbers are inherited by replacing `null` with the manifest's sequence number from the manifest list. +The `sequence_number` field represents the data sequence number and must never change after a file is added to the dataset. The data sequence number represents a relative age of the file content and should be used for planning which delete files apply to a data file. Review Comment: > The `sequence_number` field represents the data sequence number and must never change after a file is added to the dataset. this seems incorrect. data sequence number can advance with column update. -- 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]
