Re: [PR] feat(transaction): remove statistics file metadata for expired snapshots [iceberg-rust]

["amogh-jahagirdar (via GitHub)" <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #2667:
URL: https://github.com/apache/iceberg-rust/pull/2667#discussion_r3463342714


##########
crates/iceberg/src/transaction/expire_snapshots.rs:
##########
@@ -321,11 +321,31 @@ impl TransactionAction for ExpireSnapshotsAction {
             .into_iter()
             .map(|ref_name| TableUpdate::RemoveSnapshotRef { ref_name })
             .collect();
+
+        // Drop the statistics metadata tied to each expired snapshot, mirroring Java
+        // `RemoveSnapshots.removeSnapshots`. This only rewrites metadata; the puffin files those
+        // entries point at are deleted by the higher-level file-cleanup maintenance operation.
+        // Built before the `RemoveSnapshots` push so the ids can be read without cloning, but
+        // appended after it to keep the update order refs -> snapshots -> stats.

Review Comment:
   We should try and keep comments less dense imo, 
   
   ```
   // Drop statistics metadata for expired snapshots
   // This only updates metadata; puffin files are cleaned up separately.
   ```
   
   We don't need to comment the ordering details



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org