feat: add history metadata table by gmhelmold · Pull Request #2825 · apache/iceberg-rust

gmhelmold · 2026-07-13T23:12:02Z

Which issue does this PR close?

Part of #823 (metadata tables tracking issue). Follows the pattern of the existing snapshots and manifests tables.

What changes are included in this PR?

Adds the history metadata table, mirroring Java's HistoryTable:

HistoryTable in iceberg::inspect with the Java schema exactly (field ids 1–4: made_current_at timestamptz, snapshot_id, nullable parent_id, is_current_ancestor).
One row per snapshot-log entry. is_current_ancestor is computed by walking the current snapshot's parent chain — rolled-back snapshots stay visible in the log but are flagged false, matching SnapshotUtil.currentAncestorIds semantics.
parent_id is null when the entry's snapshot has been expired, matching Java's null guard.
The ancestor walk carries a cycle guard, so corrupt metadata with a parent cycle cannot hang the scan.
Registered in MetadataTableType and the DataFusion provider (table$history).

Are these changes tested?

test_history_table_with_rolled_back_snapshot: a table rolled back from S2 and re-committed as S3 (parent S1) — asserts S2 remains in history with is_current_ancestor = false while S1/S3 stay true. This lineage divergence is what distinguishes history from snapshots.
The same test also covers expired-snapshot log entries (null parent_id, false ancestor flag) and a snapshot appearing twice in the log (one row per entry).
test_history_table: schema + full-row assertions over the shared fixture.
DataFusion table listing and SHOW TABLES sqllogictest updated; public-api.txt regenerated.

laskoviymishka

This is a clean addition — it follows the snapshots/manifests sibling pattern closely, and the parts that matter for correctness line up with Java exactly: schema field ids, names, required/optional, the parent_id semantics, and made_current_at as µs timestamptz. The parent-cycle guard on the ancestor walk is a nice touch too.

Nothing here is blocking before merge. The one I care about most: that cycle guard is deliberate, but no test exercises it, so a future refactor could drop it and everything stays green. And the ancestor walk duplicates crate::util::snapshot::ancestors_of with a real divergence (the util has no cycle guard) so we should decide whether to reuse it or document why it's inlined.

A couple other smaller things:

Lock the cycle guard in with a parent-cycle metadata test asserting scan() terminates.
Reuse ancestors_of (pushing the guard down into it) or comment why the walk is inlined.
A couple of test gaps: an empty snapshot-log (zero-row batch — that's historically where Arrow schema mismatches surface) and a no-current-snapshot case (current-snapshot-id: -1 → all is_current_ancestor false).
Minor consistency, all optional: UTC_TIME_ZONE instead of the "+00:00" literal, field-vs-method access on entry, and a checked multiply on timestamp_ms * 1000.

Most of the small stuff is shared debt with the sibling tables, so no strong feelings there. Will wait @CTTY before merge.

And thanks for this amazing contribution!

laskoviymishka · 2026-07-14T08:11:07Z

+        // is what tells the live lineage apart from rolled-back history entries.
+        let mut ancestors = HashSet::new();
+        let mut snapshot = metadata.current_snapshot();
+        while let Some(ancestor) = snapshot {


This ancestor walk is basically crate::util::snapshot::ancestors_of (util/snapshot.rs:38) inlined — except that one has no cycle guard and this one does.

I'd rather not maintain two copies of the traversal that disagree on a safety property. Could we push the cycle guard down into ancestors_of and reuse it here? If there's a real API mismatch (&TableMetadata vs &TableMetadataRef) that makes reuse awkward, a one-line comment noting why it's inlined would do. wdyt?

laskoviymishka · 2026-07-14T08:11:07Z

+        let mut ancestors = HashSet::new();
+        let mut snapshot = metadata.current_snapshot();
+        while let Some(ancestor) = snapshot {
+            // corrupt metadata with a parent cycle must not hang the scan


This guard is the one thing here that isn't exercised by the tests, and it's exactly the kind of thing a refactor removes silently. I'd add a metadata fixture with a parent cycle and assert scan() terminates, so the guard is actually locked in.

laskoviymishka · 2026-07-14T08:11:07Z

+        let mut is_current_ancestor = BooleanBuilder::new();
+
+        for entry in metadata.history() {
+            made_current_at.append_value(entry.timestamp_ms() * 1000); // ms -> µs


timestamp_ms() * 1000 is an unchecked i64 multiply — it'll wrap for pre-epoch or huge values. snapshots.rs:112 has the same thing, so this isn't a regression, but a saturating/checked multiply would be more defensive if you're touching it anyway.

laskoviymishka · 2026-07-14T08:11:07Z

+
+        for entry in metadata.history() {
+            made_current_at.append_value(entry.timestamp_ms() * 1000); // ms -> µs
+            snapshot_id.append_value(entry.snapshot_id);


Small consistency thing: entry.timestamp_ms() uses the method one line up and entry.snapshot_id uses the field here. SnapshotLog exposes both, so it works, but reading them back to back it's a little jarring. I'd use field access for both — entry.timestamp_ms.

laskoviymishka · 2026-07-14T08:11:07Z

+        }
+
+        let mut made_current_at =
+            PrimitiveBuilder::<TimestampMicrosecondType>::new().with_timezone("+00:00");


There's a crate::arrow::UTC_TIME_ZONE constant (arrow/schema.rs:45) for exactly this. snapshots.rs:86 also hard-codes the string, so this'd be the second occurrence — I'd use the constant here rather than grow the string literals.

laskoviymishka · 2026-07-14T08:11:07Z

+        // Walk the current snapshot's parent chain once: membership in this set
+        // is what tells the live lineage apart from rolled-back history entries.
+        let mut ancestors = HashSet::new();
+        let mut snapshot = metadata.current_snapshot();


Heads up that current_snapshot() has a hidden .expect() (table_metadata.rs:315) that panics if current_snapshot_id is set but the snapshot is missing from the map — corrupt or GC'd metadata. validate_refs probably makes that unreachable for a well-formed Table, so I don't think it needs code here, but a short comment noting we rely on that invariant would save the next reader the trip. (Java's SnapshotUtil returns null instead of panicking, fwiw.)

gmhelmold · 2026-07-14T08:47:52Z

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

Add an inspect subsystem (Table.Inspect) and its first metadata table, History, mirroring the Java, PyIceberg, and Rust clients (apache/iceberg-rust#2825). History exposes the snapshot log as an Arrow RecordReader with columns made_current_at (timestamp[us,UTC]), snapshot_id, parent_id, and is_current_ancestor (field IDs 1-4 for cross-client parity). Ancestry is computed by walking the current snapshot's parent chain via the existing AncestorsOf helper (cycle-guarded); rolled-back snapshots are flagged non-ancestors and expired snapshots render a null parent_id. The context is honored during iteration and errors are wrapped.

feat: add history metadata table

2a9147a

laskoviymishka approved these changes Jul 14, 2026

View reviewed changes

refactor: reuse ancestors_of with cycle guard and extend history tests

5c544c4

Merge branch 'main' into feat/history-metadata-table

7f21b27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add history metadata table#2825

feat: add history metadata table#2825
gmhelmold wants to merge 3 commits into
apache:mainfrom
gmhelmold:feat/history-metadata-table

gmhelmold commented Jul 13, 2026

Uh oh!

laskoviymishka left a comment

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

laskoviymishka Jul 14, 2026

Uh oh!

gmhelmold commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gmhelmold commented Jul 13, 2026

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

Uh oh!

laskoviymishka left a comment

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

laskoviymishka Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

gmhelmold commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants