rdblue commented on code in PR #16025:
URL: https://github.com/apache/iceberg/pull/16025#discussion_r4224433311


##########
format/spec.md:
##########
@@ -731,42 +748,150 @@ The `data_file` struct consists of the following fields:
     | _optional_ | _optional_ | _optional_ | **`110  null_value_counts`**      
| `map<121: int, 122: long>`                                                  | 
Map from column id to number of null values in the column |
     | _optional_ | _optional_ | _optional_ | **`137  nan_value_counts`**       
| `map<138: int, 139: long>`                                                  | 
Map from column id to number of NaN values in the column |
     | _optional_ | _optional_ |            | ~~**`111  distinct_counts`**~~    
| `map<123: int, 124: long>`                                                  | 
**Deprecated. Do not write.** |
-    | _optional_ | _optional_ | _optional_ | **`125  lower_bounds`**           
| `map<126: int, 127: binary>`                                                | 
Map from column id to lower bound in the column serialized as binary [1]. Each 
value must be less than or equal to all non-null, non-NaN values in the column 
for the file [2] |
-    | _optional_ | _optional_ | _optional_ | **`128  upper_bounds`**           
| `map<129: int, 130: binary>`                                                | 
Map from column id to upper bound in the column serialized as binary [1]. Each 
value must be greater than or equal to all non-null, non-Nan values in the 
column for the file [2] |
+    | _optional_ | _optional_ | _optional_ | **`125  lower_bounds`**           
| `map<126: int, 127: binary>`                                                | 
Map from column id to lower bound in the column serialized as binary. Each 
value must be less than or equal to all non-null, non-NaN values in the column 
for the file. See [Field-level Metrics and 
Statistics](#field-level-metrics-and-statistics) |
+    | _optional_ | _optional_ | _optional_ | **`128  upper_bounds`**           
| `map<129: int, 130: binary>`                                                | 
Map from column id to upper bound in the column serialized as binary. Each 
value must be greater than or equal to all non-null, non-Nan values in the 
column for the file. See [Field-level Metrics and 
Statistics](#field-level-metrics-and-statistics) |
     | _optional_ | _optional_ | _optional_ | **`131  key_metadata`**           
| `binary`                                                                    | 
Implementation-specific key metadata for encryption |
     | _optional_ | _optional_ | _optional_ | **`132  split_offsets`**          
| `list<133: long>`                                                           | 
Split offsets for the data file. For example, all row group offsets in a 
Parquet file. Must be sorted ascending |
     |            | _optional_ | _optional_ | **`135  equality_ids`**           
| `list<136: int>`                                                            | 
Field ids used to determine row equality in equality delete files. Required 
when `content=2` and should be null otherwise. Fields with ids listed in this 
column must be present in the delete file |
-    | _optional_ | _optional_ | _optional_ | **`140  sort_order_id`**          
| `int`                                                                       | 
ID representing sort order for this file [3]. |
+    | _optional_ | _optional_ | _optional_ | **`140  sort_order_id`**          
| `int`                                                                       | 
ID representing sort order for this file [1]. |
     |            |            | _optional_ | **`142  first_row_id`**           
| `long`                                                                      | 
The `_row_id` for the first row in the data file. See [First Row ID 
Inheritance](#first-row-id-inheritance) |
-    |            | _optional_ | _optional_ | **`143  referenced_data_file`**   
| `string`                                                                    | 
Fully qualified location (URI with FS scheme) of a data file that all deletes 
reference [4] |
-    |            |            | _optional_ | **`144  content_offset`**         
| `long`                                                                      | 
The offset in the file where the content starts [5] |
-    |            |            | _optional_ | **`145  content_size_in_bytes`**  
| `long`                                                                      | 
The length of a referenced content stored in the file; required if 
`content_offset` is present [5] |
+    |            | _optional_ | _optional_ | **`143  referenced_data_file`**   
| `string`                                                                    | 
Fully qualified location (URI with FS scheme) of a data file that all deletes 
reference [2] |
+    |            |            | _optional_ | **`144  content_offset`**         
| `long`                                                                      | 
The offset in the file where the content starts [3] |
+    |            |            | _optional_ | **`145  content_size_in_bytes`**  
| `long`                                                                      | 
The length of a referenced content stored in the file; required if 
`content_offset` is present [3] |
 
-The `partition` struct stores the tuple of partition values for each file. Its 
type is derived from the partition fields of the partition spec used to write 
the manifest file. In v2, the partition struct's field ids must match the ids 
from the partition spec.
+    The `partition` struct stores the tuple of partition values for each file. 
Its type is derived from the partition fields of the partition spec used to 
write the manifest file. In v2, the partition struct's field ids must match the 
ids from the partition spec.
 
-The v4 `content_stats` container struct stores field-level metrics. Unlike the 
metrics maps, the type of `content_stats` is based on table metadata, like 
schema. Similar to the `partition` struct, the same type is used for all files 
tracked in a manifest.
+    Notes:
 
-Notes:
+    1. If sort order ID is missing or unknown, then the order is assumed to be 
unsorted. Only data files and equality delete files should be written with a 
non-null order id. [Position deletes](#position-delete-files) are required to 
be sorted by file and position, not a table order, and should set sort order id 
to null. Readers must ignore sort order id for position delete files.
+    2. Position delete metadata can use `referenced_data_file` when all 
deletes tracked by the entry are in a single data file. Setting the referenced 
file is required for deletion vectors.
+    3. The `content_offset` and `content_size_in_bytes` fields are used to 
reference a specific blob for direct access to a deletion vector. For deletion 
vectors, these values are required and must exactly match the `offset` and 
`length` stored in the Puffin footer for the deletion vector blob.
+    4. The following field ids are reserved on `data_file`: 141.
 
-1. Single-value serialization for lower and upper bounds is detailed in 
Appendix D.
-2. For `float` and `double`, the value `-0.0` must precede `+0.0`, as in the 
IEEE 754 `totalOrder` predicate. NaNs are not permitted as lower or upper 
bounds.
-3. If sort order ID is missing or unknown, then the order is assumed to be 
unsorted. Only data files and equality delete files should be written with a 
non-null order id. [Position deletes](#position-delete-files) are required to 
be sorted by file and position, not a table order, and should set sort order id 
to null. Readers must ignore sort order id for position delete files.
-4. Position delete metadata can use `referenced_data_file` when all deletes 
tracked by the entry are in a single data file. Setting the referenced file is 
required for deletion vectors.
-5. The `content_offset` and `content_size_in_bytes` fields are used to 
reference a specific blob for direct access to a deletion vector. For deletion 
vectors, these values are required and must exactly match the `offset` and 
`length` stored in the Puffin footer for the deletion vector blob.
-6. The following field ids are reserved on `data_file`: 141.
+=== "v4"
+    The `tracked_file` struct has the following fields:
+
+    | On write   | Field id | Name                     | Type                  
                                | Description |
+    
|------------|----------|--------------------------|-------------------------------------------------------|-------------|
+    | _required_ | 134      | **`content_type`**       | `int` (0: DATA, 3: 
DATA_MANIFEST, 4: DELETE_MANIFEST) | Type of content stored in the entry. |
+    | _required_ | 147      | **`tracking`**           | `tracking` struct     
                                | Tracking metadata like status, snapshot ID, 
and sequence number. See [Tracking](#tracking). |
+    | _required_ | 100      | **`location`**           | `string`              
                                | Location of the file. |
+    | _required_ | 101      | **`file_format`**        | `string`              
                                | String file format name: `avro`, `orc`, or 
`parquet` |
+    | _required_ | 104      | **`file_size_in_bytes`** | `long`                
                                | Total file size in bytes. |
+    | _required_ | 103      | **`record_count`**       | `long`                
                                | Number of records in this file. |
+    | _optional_ | 131      | **`key_metadata`**       | `binary`              
                                | Key metadata for encryption; specific to the 
encryption scheme. |
+    | _optional_ | 132      | **`split_offsets`**      | `list<133: long>`     
                                | Split offsets for the data file. Must be 
sorted ascending. |
+    | _optional_ | 141      | **`spec_id`**            | `int`                 
                                | ID of the partition spec used to partition 
the file; null if unpartitioned |
+    | _optional_ | 102      | **`partition`**          | `struct<...>`         
                                | Partition data tuple for the file; null if 
unpartitioned. |
+    | _optional_ | 140      | **`sort_order_id`**      | `int`                 
                                | ID representing sort order for this file. If 
missing or unknown, the order is assumed to be unsorted. |
+    | _optional_ | 146      | **`content_stats`**      | `content_stats` 
struct                                | Field-level stats. See [Content 
Stats](#content-stats). |
+    | _optional_ | 150      | **`manifest_info`**      | `manifest_info` 
struct                                | Manifest-specific stats. See [Manifest 
Info](#manifest-info). |
+    | _optional_ | 148      | **`deletion_vector`**    | `deletion_vector` 
struct                              | Row-level deletion vector for a data 
file. See [Deletion Vector](#deletion-vector). |
+    | _optional_ | 158      | **`column_files`**       | `list<159: 
column_file>`                              | Column files associated with this 
file. See [Column File](#column-file). |
+
+    ##### Tracking
+
+    The `tracking` struct has the following fields:
+
+    | On write   | Field id | Name                          | Type             
                                                   | Description |
+    
|------------|----------|-------------------------------|---------------------------------------------------------------------|-------------|
+    | _required_ | 0        | **`status`**                  | `int` (0: 
EXISTING, 1: ADDED, 2: DELETED, 3: REPLACED, 4: MODIFIED) | Used to track 
additions, deletions, replacements, and modifications. |
+    | _optional_ | 1        | **`snapshot_id`**             | `long`           
                                                   | Snapshot ID where the file 
was added, replaced, or deleted. Inherited when null. |
+    | _optional_ | 5        | **`modified_snapshot_id`**    | `long`           
                                                   | Snapshot ID where the file 
was last modified. |
+    | _optional_ | 3        | **`sequence_number`**         | `long`           
                                                   | Data sequence number of 
the file. Inherited when null. See [Sequence Number 
Inheritance](#sequence-number-inheritance). |
+    | _optional_ | 4        | **`file_sequence_number`**    | `long`           
                                                   | File sequence number 
indicating when the file was added. Inherited when null. See [Sequence Number 
Inheritance](#sequence-number-inheritance). |
+    | _optional_ | 142      | **`first_row_id`**            | `long`           
                                                   | Base row ID for assigning 
`_row_id` values. See [First Row ID Inheritance](#first-row-id-inheritance). |
+    | _optional_ | 6        | **`deleted_positions`**       | `binary`         
                                                   | Positions deleted via 
manifest DV in the `modified_snapshot_id` snapshot. See [Manifest Deletion 
Vectors](#manifest-deletion-vectors). |
+    | _optional_ | 7        | **`replaced_positions`**      | `binary`         
                                                   | Positions replaced via 
manifest DV in the `modified_snapshot_id` snapshot. See [Manifest Deletion 
Vectors](#manifest-deletion-vectors). |
+
+    ##### Deletion Vector
+
+    The `deletion_vector` struct has the following fields:
+
+    | On write   | Field id | Name                | Type     | Description |
+    |------------|----------|---------------------|----------|-------------|
+    | _required_ | 155      | **`location`**      | `string` | Location of the 
file that stores the DV. |
+    | _required_ | 144      | **`offset`**        | `long`   | Offset in the 
file where the content starts. |
+    | _required_ | 145      | **`size_in_bytes`** | `long`   | Length of the 
referenced content stored in the file. |
+    | _required_ | 156      | **`cardinality`**   | `long`   | Number of set 
bits (deleted rows) in the deletion vector. |
+    | _optional_ | 149      | **`key_metadata`**  | `binary` | Key metadata 
for encryption; specific to the encryption scheme. |
+
+    ##### Manifest Info
+
+    The `manifest_info` struct has the following fields:
+
+    | On write   | Field id | Name                       | Type                
     | Description |
+    
|------------|----------|----------------------------|--------------------------|-------------|
+    | _required_ | 521      | **`format_version`**       | `int` (0: PRE-V4, 
4: V4) | Format version used to write the manifest. |
+    | _optional_ | 522      | **`dv`**                   | `binary`            
     | Positions in the referenced leaf manifest that are not live. See 
[Manifest Deletion Vectors](#manifest-deletion-vectors). |
+    | _required_ | 504      | **`added_files_count`**    | `int`               
     | Count of entries with status ADDED in the manifest. |
+    | _required_ | 505      | **`existing_files_count`** | `int`               
     | Count of entries with status EXISTING in the manifest. |
+    | _required_ | 525      | **`modified_files_count`** | `int`               
     | Count of entries with status MODIFIED in the manifest. |
+    | _required_ | 506      | **`deleted_files_count`**  | `int`               
     | Count of entries with status DELETED in the manifest. |
+    | _required_ | 523      | **`replaced_files_count`** | `int`               
     | Count of entries with status REPLACED in the manifest. |
+    | _required_ | 512      | **`added_rows_count`**     | `long`              
     | Total number of rows in ADDED entries. |
+    | _required_ | 513      | **`existing_rows_count`**  | `long`              
     | Total number of rows in EXISTING entries. |
+    | _required_ | 526      | **`modified_rows_count`**  | `long`              
     | Total number of rows in MODIFIED entries. |
+    | _required_ | 514      | **`deleted_rows_count`**   | `long`              
     | Total number of rows in DELETED entries. |
+    | _required_ | 524      | **`replaced_rows_count`**  | `long`              
     | Total number of rows in REPLACED entries. |
+    | _required_ | 516      | **`min_sequence_number`**  | `long`              
     | Minimum data sequence number of all live entries in the manifest. |
+
+    ##### Column File
+
+    The `column_file` struct has the following fields:
+
+    | On write   | Field id | Name                     | Type             | 
Description |
+    
|------------|----------|--------------------------|------------------|-------------|
+    | _required_ | 160      | **`location`**           | `string`         | 
Location of the column file. |
+    | _required_ | 161      | **`field_ids`**          | `list<162: int>` | 
Live field IDs stored in this column file. |
+    | _required_ | 163      | **`file_format`**        | `string`         | 
String file format name: `avro`, `orc`, or `parquet`. |
+    | _required_ | 164      | **`file_size_in_bytes`** | `long`           | 
Total column file size in bytes. |
+    | _optional_ | 165      | **`key_metadata`**       | `binary`         | 
Key metadata for encryption; specific to the encryption scheme. |
+
+    ##### Tracked File Requirements
+
+    - `deletion_vector.offset` and `deletion_vector.size_in_bytes` must 
exactly match the `offset` and `length` stored in the Puffin footer for the 
deletion vector blob.
+    - A leaf manifest written in v4 may only contain data files.
+    - Row-level deletes may only be written in v4 as deletion vectors in the 
data file's `deletion_vector`.
+    - Delete files from pre-v4 tables are valid in upgraded tables and are 
tracked in delete manifests written before the upgrade.
+    - A root manifest may contain manifests from any format version, but only 
v4 leaf manifests may be created by writers.
+    - `manifest_info.format_version` must be V4 for manifests written in v4 
and PRE-V4 for manifests written by earlier versions.
+    - `manifest_info` must be set if and only if the tracked file is a 
manifest.
+    - For manifests, `manifest_info.added_files_count`, 
`existing_files_count`, `deleted_files_count`, `replaced_files_count`, and 
`modified_files_count` must sum to `record_count`.
+    - `deletion_vector` may only be set if the tracked file is a data file.
+    - `column_files` may only be set if the tracked file is a data file or a 
v4 leaf manifest.
+    - A field ID may appear in `field_ids` of at most one column file in a 
tracked file's `column_files`.
+    - `tracking.deleted_positions` and `tracking.replaced_positions` may only 
be set if the tracked file is a manifest.
+    - `tracking.snapshot_id`, `tracking.sequence_number`, and 
`tracking.file_sequence_number` are required for all tracked files in the root 
manifest. `tracking.first_row_id` is also required for data files, data 
manifests, and v4 leaf manifests in the root manifest.
+    - Writers should not write a null `tracking.snapshot_id`.
+    - For manifests, `spec_id` must be set to the `spec_id` of the manifest's 
entries if all entries have the same `spec_id`, and must be null otherwise.
+
+    ###### Updating Tracking Metadata
+
+    When a file is added to the dataset, its tracked file must set status to 
ADDED and store the snapshot ID in which the file was added.
+
+    When a data file's deletion vector or column files are updated, the writer 
must produce two entries: a MODIFIED entry for the updated live version and a 
REPLACED entry with the previous metadata (for change detection). The MODIFIED 
entry's `modified_snapshot_id` must record the snapshot ID in which the change 
occurred. The REPLACED entry can be produced in place by setting its position 
in the leaf manifest's [`manifest_info.dv`](#manifest-deletion-vectors) (to 
remove it from planning) and `tracking.replaced_positions` (for change 
detection). The REPLACED entry's `snapshot_id` must record the snapshot where 
the entry was replaced. For an entry replaced using 
`tracking.replaced_positions`, the snapshot where it was replaced is the leaf 
manifest's `modified_snapshot_id`. When writing an existing file to a new 
manifest or marking an existing file as deleted, its `modified_snapshot_id` 
must be preserved.
+
+    When a file is deleted from the dataset, the deletion must be recorded in 
the snapshot that deletes the file with a DELETED entry that stores the 
snapshot ID in which the file was deleted or, for an entry in a leaf manifest, 
alternatively by setting its position in the leaf manifest's 
`tracking.deleted_positions` and 
[`manifest_info.dv`](#manifest-deletion-vectors) and updating 
`tracking.modified_snapshot_id` to the new snapshot ID.
+
+    A leaf manifest whose `manifest_info.dv` changed must have status 
MODIFIED. `tracking.deleted_positions` and `tracking.replaced_positions` should 
only be set in the snapshot that changes `manifest_info.dv`.

Review Comment:
   ```suggestion
       A leaf manifest whose `manifest_info.dv` changed must have status 
MODIFIED and records the snapshot in its `modified_snapshot_id`. When new 
positions are set in `manifest_info.dv`, those positions must be set in either 
`tracking.deleted_positions` or `tracking.replaced_positions` to record what 
the entry's state changed to in the snapshot identified by 
`modified_snapshot_id`. Deleted and replaced bitmaps may be removed when the 
manifest metadata is copied to a new root.
   ```



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