amogh-jahagirdar commented on code in PR #16025:
URL: https://github.com/apache/iceberg/pull/16025#discussion_r4233621971


##########
format/spec.md:
##########
@@ -656,15 +661,29 @@ A data or delete file is associated with a sort order by 
the sort order's id wit
 
 ### Manifests
 
-A manifest is an immutable Avro file that lists data files or delete files, 
along with each file’s partition data tuple, metrics, and tracking information. 
One or more manifest files are used to store a [snapshot](#snapshots), which 
tracks all of the files in a table at some point in time. Manifests are tracked 
by a [manifest list](#manifest-lists) for each table snapshot.
+A table's metadata tree is composed of manifests. A manifest is an immutable 
file that tracks a subset of a table metadata for a given 
[snapshot](#snapshots). Leaf manifests are the lowest level of the metadata 
tree and track data or delete files, along with each file's partition data, 
metrics, and tracking information. The snapshot root file tracks leaf 
manifests; in v4 and later the root is also a manifest and can also track data 
files.
 
 A manifest is a valid Iceberg data file: files must use valid Iceberg formats, 
schemas, and column projection.
 
-A manifest may store either data files or delete files, but not both because 
manifests that contain delete files are scanned first during job planning. 
Whether a manifest is a data manifest or a delete manifest is stored in 
manifest metadata.
+Each manifest type contains the following content:
 
-A manifest stores files for a single partition spec. When a table’s partition 
spec changes, old files remain in the older manifest and newer files are 
written to a new manifest. This is required because a manifest file’s schema is 
based on its partition spec (see below). The partition spec of each manifest is 
also used to transform predicates on the table's data rows into predicates on 
partition values that are used during job planning to select files from a 
manifest.
+| Version | Manifest type   | Contents                                        
| File format |
+|---------|-----------------|-------------------------------------------------|-------------|
+| v1-v3   | Data manifest   | Data files                                      
| Avro        |
+| v2-v3   | Delete manifest | Delete files                                    
| Avro        |
+| v4      | Root manifest   | Leaf manifests, data files, or v1-v3 manifests  
| Parquet     |
+| v4      | Leaf manifest   | Data files and their colocated deletion vectors 
| Parquet     |
 
-A manifest file must store the partition spec and other metadata as properties 
in the Avro file's key-value metadata:
+In v2-v3, whether a manifest is a data manifest or a delete manifest is stored 
in manifest metadata.
+
+- v1-v3: A manifest stores files for a single partition spec. When a table’s 
partition spec changes, old files remain in the older manifest and newer files 
are written to a new manifest. This is required because a manifest file’s 
schema is based on its partition spec.
+- v4: A manifest may store files written with different partition specs.
+
+The partition spec used when writing each data file is used to transform 
predicates on the table’s data rows into predicates on partition values during 
job planning.

Review Comment:
   Sorry I'm not sure I follow, which part isn't true? The sentence says the 
spec used to write each data file is used to project the predicates, which I 
think is what you're saying.



##########
format/spec.md:
##########
@@ -656,15 +661,29 @@ A data or delete file is associated with a sort order by 
the sort order's id wit
 
 ### Manifests
 
-A manifest is an immutable Avro file that lists data files or delete files, 
along with each file’s partition data tuple, metrics, and tracking information. 
One or more manifest files are used to store a [snapshot](#snapshots), which 
tracks all of the files in a table at some point in time. Manifests are tracked 
by a [manifest list](#manifest-lists) for each table snapshot.
+A table's metadata tree is composed of manifests. A manifest is an immutable 
file that tracks a subset of a table metadata for a given 
[snapshot](#snapshots). Leaf manifests are the lowest level of the metadata 
tree and track data or delete files, along with each file's partition data, 
metrics, and tracking information. The snapshot root file tracks leaf 
manifests; in v4 and later the root is also a manifest and can also track data 
files.
 
 A manifest is a valid Iceberg data file: files must use valid Iceberg formats, 
schemas, and column projection.
 
-A manifest may store either data files or delete files, but not both because 
manifests that contain delete files are scanned first during job planning. 
Whether a manifest is a data manifest or a delete manifest is stored in 
manifest metadata.
+Each manifest type contains the following content:
 
-A manifest stores files for a single partition spec. When a table’s partition 
spec changes, old files remain in the older manifest and newer files are 
written to a new manifest. This is required because a manifest file’s schema is 
based on its partition spec (see below). The partition spec of each manifest is 
also used to transform predicates on the table's data rows into predicates on 
partition values that are used during job planning to select files from a 
manifest.
+| Version | Manifest type   | Contents                                        
| File format |
+|---------|-----------------|-------------------------------------------------|-------------|
+| v1-v3   | Data manifest   | Data files                                      
| Avro        |
+| v2-v3   | Delete manifest | Delete files                                    
| Avro        |
+| v4      | Root manifest   | Leaf manifests, data files, or v1-v3 manifests  
| Parquet     |
+| v4      | Leaf manifest   | Data files and their colocated deletion vectors 
| Parquet     |
 
-A manifest file must store the partition spec and other metadata as properties 
in the Avro file's key-value metadata:
+In v2-v3, whether a manifest is a data manifest or a delete manifest is stored 
in manifest metadata.

Review Comment:
   Done!



##########
format/spec.md:
##########
@@ -676,15 +695,26 @@ A manifest file must store the partition spec and other 
metadata as properties i
     | _optional_ | _required_ | `format-version`    | Table format version 
number of the manifest as a string                                              
                                       |
     |            | _required_ | `content`           | Type of content files 
tracked by the manifest: "data" or "deletes"                                    
                                      |
 
+=== "v4"
+    | Requirement | Key                 | Value                                
                                                                                
                       |
+    
|-------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------|
+    | _required_  | `schema-id`         | ID of the schema used to write the 
manifest as a string                                                            
                         |
+    | _required_  | `format-version`    | Table format version number of the 
manifest as a string                                                            
                         |

Review Comment:
   Done!



##########
format/spec.md:
##########
@@ -676,15 +695,26 @@ A manifest file must store the partition spec and other 
metadata as properties i
     | _optional_ | _required_ | `format-version`    | Table format version 
number of the manifest as a string                                              
                                       |
     |            | _required_ | `content`           | Type of content files 
tracked by the manifest: "data" or "deletes"                                    
                                      |
 
+=== "v4"
+    | Requirement | Key                 | Value                                
                                                                                
                       |
+    
|-------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------|
+    | _required_  | `schema-id`         | ID of the schema used to write the 
manifest as a string                                                            
                         |
+    | _required_  | `format-version`    | Table format version number of the 
manifest as a string                                                            
                         |
+
 #### 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.
 
-#### Manifest Entry Fields
+#### Manifest Schema
 
-The schema of a manifest file is defined by the `manifest_entry` struct, which 
consists of the following fields:
+In v4, manifests store `tracked_file` records that describe data or manifest 
files and contain `tracking` metadata, like `status`. The relationship between 
a file and its tracking metadata is inverted in v1-v3 manifests, which store 
`manifest_entry` records that contain tracking information and a `data_file` 
struct.

Review Comment:
   Done, added that the v4 manifest schema is not compatible with the v1-v3 
manifest schema.



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

Review Comment:
   Removed them and used hidden anchors instead so the links still work. The 
struct ones have a `-fields` suffix like the v1-v3 tab so they don't conflict 
with existing headings like Deletion Vectors. I kept the structs in the v4 tab 
though so someone implementing a manifest reader has everything in one place.



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