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


##########
format/spec.md:
##########
@@ -731,39 +744,151 @@ 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:
+
+    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.
+
+=== "v4"

Review Comment:
   One high level thing I would like to see here is the differences between 
root and leaf manifests. Leaf manifests cannot store other manifests so they 
don't have `manifest_info` and may (must?) also omit the `tracking.dv`, 
`tracking.deleted_positions`, and `tracking.replaced_positions` fields from the 
write schema. Maybe the table should have a column for each writer requirement?
   
   In addition, we need to note that snapshot ID, sequence numbers, and first 
row ID are required for all entries in the root manifest. That means these are 
_required_ fields in the root write schema, but not in leaf write schemas.



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