singhpk234 commented on code in PR #17533:
URL: https://github.com/apache/iceberg/pull/17533#discussion_r3739095126


##########
format/spec.md:
##########
@@ -1209,15 +1207,16 @@ many statistics files associated with different table 
snapshots.
 
 Statistics files metadata within `statistics` table metadata field is a struct 
with the following fields:
 
-=== "v1 - v3"
-    | v1         | v2 and v3  | Field name                      | Type         
         | Description |
-    | ---------- | ---------- 
|---------------------------------|-----------------------|-------------|
-    | _required_ | _required_ | **`snapshot-id`**               | `long`       
         | ID of the Iceberg table's snapshot the statistics file is associated 
with. |
-    | _required_ | _required_ | **`statistics-path`**           | `string`     
         | Path of the statistics file. See [Puffin file 
format](puffin-spec.md). |
-    | _required_ | _required_ | **`file-size-in-bytes`**        | `long`       
         | Size of the statistics file. |
-    | _required_ | _required_ | **`file-footer-size-in-bytes`** | `long`       
         | Total size of the statistics file's footer (not the footer payload 
size). See [Puffin file format](puffin-spec.md) for footer definition. |
-    | _optional_ | _optional_ | **`key-metadata`**              |              
         | Base64-encoded implementation-specific key metadata for encryption. |
-    | _required_ | _required_ | **`blob-metadata`**             | `list<blob 
metadata>` (see below) | A list of the blob metadata for statistics contained 
in the file with structure described below. |
+=== "v1 - v4"
+    | v1 and v2  | v3         | v4         | Field name                      | 
Type                  | Description |
+    | ---------- | ---------- | ---------- 
|---------------------------------|-----------------------|-------------|
+    | _required_ | _required_ | _required_ | **`snapshot-id`**               | 
`long`                | ID of the Iceberg table's snapshot the statistics file 
is associated with. |
+    | _required_ | _required_ | _required_ | **`statistics-path`**           | 
`string`              | Path of the statistics file. See [Puffin file 
format](puffin-spec.md). |
+    | _required_ | _required_ | _required_ | **`file-size-in-bytes`**        | 
`long`                | Size of the statistics file. |
+    | _required_ | _required_ | _required_ | **`file-footer-size-in-bytes`** | 
`long`                | Total size of the statistics file's footer (not the 
footer payload size). See [Puffin file format](puffin-spec.md) for footer 
definition. |
+    | _optional_ | _optional_ |            | ~~**`key-metadata`**~~          | 
                      | Base64-encoded implementation-specific key metadata for 
encryption. (**Deprecated**: use `key-id` instead) |
+    | _optional_ | _optional_ | _optional_ | **`key-id`**                    | 
`string`              | ID of the encryption key that encrypts the statistics 
file key metadata |

Review Comment:
   Should it be the current KEK or the KEK of the snapshot MLK was encrypted 
with ? there is possibility that a new KEK is created before we ran an offline 
process to collect stats ? in that case we use KEK of the MLK for snapshot != 
KEK of the partition file. 
   I think it should be fine i think but i want to bring this up for discussion 



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