gmhelmold commented on PR #2825: URL: https://github.com/apache/iceberg-rust/pull/2825#issuecomment-4967096925
Thanks for the thorough review! All addressed in 5c544c4: - **Traversal unified**: the cycle guard is pushed down into `Ancestors::next` in `util/snapshot.rs`, and `history.rs` now uses `ancestors_of` — one traversal, one termination property (the `ancestors_of` doc states it now). - **Guard locked in**: `test_history_table_with_parent_cycle` builds metadata with S1/S2 claiming each other as parent and asserts the scan terminates — verified that removing the guard makes this test hang, so a refactor can't drop it silently. - **Empty snapshot-log** and **no-current-snapshot** (`current-snapshot-id: -1` → all `is_current_ancestor` false) both covered with full-schema assertions. - Minors: `UTC_TIME_ZONE` constant, consistent field access on `entry`, and `saturating_mul(1000)` for the ms→µs conversion (kept scoped to this file — happy to sweep `snapshots.rs` in a follow-up if wanted). `public-api.txt` is unchanged (verified against a regen). -- 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]
