feat(inspect): implement streaming SnapshotsTable scans by WZhuo · Pull Request #801 · apache/iceberg-cpp

WZhuo · 2026-07-02T08:35:38Z

Summary

Implement streaming scans for SnapshotsTable and refine the metadata-table scan APIs so snapshot metadata can be consumed safely in bounded Arrow batches.

Changes

Metadata table APIs

Add a typed MetadataTable::Make<T>() factory that preserves the concrete metadata-table type.
Keep the base MetadataTable interface limited to non-time-travel Scan() calls.
Add TimeTravelMetadataTable as the capability-specific interface for scans using SnapshotSelection.
Model snapshot ID and as-of timestamp as mutually exclusive values with std::variant<std::monostate, int64_t, TimePointMs>.
Return ArrowArrayStream from scan APIs and document stream ownership.
Provide shared metadata-table schema and source-table accessors.

Snapshots table

Implement SnapshotsTable::Scan() using a stateful Arrow stream.
Convert the Iceberg schema to ArrowSchema once when creating the stream and release owned resources when the stream closes.
Emit at most MetadataTable::kBatchSize rows per batch.
Skip null snapshot entries safely.
Materialize committed_at, snapshot_id, parent_id, operation, manifest_list, and summary using ArrowRowBuilder.
Remove SnapshotSummaryFields::kOperation from the summary map and emit null when the remaining summary is empty.

Arrow row builder

Add ArrowRowBuilder::num_rows() so streaming producers can enforce batch-size limits without maintaining duplicate row counters.

Tests

Add shared metadata-table fixtures and Arrow stream import helpers.
Cover snapshots schema and row values, summary filtering, null snapshots, empty summaries, empty tables, and multi-batch scans.
Cover metadata-table construction, time-travel capability reporting, and history-table schema behavior.

Testing

cmake --build build --target metadata_table_test -j2
build/src/iceberg/test/metadata_table_test

Copilot

Pull request overview

Implements initial metadata-table scanning support by adding a Scan() API to MetadataTable (with snapshot-selection parameters for future time-travel) and providing a concrete SnapshotsTable::Scan() implementation that materializes snapshot rows into Arrow arrays. The PR also restructures/extends the metadata-table test suite to validate schemas and snapshot scanning behavior.

Changes:

Added SnapshotSelection and a virtual MetadataTable::Scan() API (with a convenience overload) plus supports_time_travel().
Implemented SnapshotsTable::Scan() to emit snapshot rows (6 columns) via ArrowRowBuilder.
Added/expanded tests and wired new test sources into the metadata-table test target.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/iceberg/inspect/metadata_table.h	Adds `SnapshotSelection`, `Scan()` API, and `supports_time_travel()` declaration/docs.
src/iceberg/inspect/metadata_table.cc	Implements default `Scan()` + `supports_time_travel()` and wires factory for kinds.
src/iceberg/inspect/snapshots_table.h	Declares `SnapshotsTable::Scan()` override.
src/iceberg/inspect/snapshots_table.cc	Implements snapshot scanning into Arrow via `ArrowRowBuilder`.
src/iceberg/test/metadata_table_test.cc	Simplifies base setup and adds `SupportsTimeTravel` test.
src/iceberg/test/metadata_table_test_base.h	New shared fixture/helpers for metadata table tests.
src/iceberg/test/snapshots_table_test.cc	New tests validating snapshots table construction/schema/scan output.
src/iceberg/test/history_table_test.cc	New schema test for history table.
src/iceberg/test/CMakeLists.txt	Adds new test sources to the metadata-table test target.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/iceberg/inspect/metadata_table.cc:38

supports_time_travel() is currently hard-coded to return false. That matches today’s two Kind values, but it’s easy to forget to update once additional metadata table kinds are added, and it doesn’t reflect the docstring/PR description that this is kind-driven. Consider switching on kind() and making the non-exhaustive case unreachable to keep future additions honest.

bool MetadataTable::supports_time_travel() const noexcept { return false; }

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/iceberg/inspect/metadata_table.cc:38

supports_time_travel() is documented (and described in the PR) as being driven by the metadata table kind, but the implementation always returns false regardless of kind. This makes the API misleading and forces future kinds to remember to update callers rather than the method itself.

bool MetadataTable::supports_time_travel() const noexcept { return false; }

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

manuzhang · 2026-09-03T08:59:29Z

@WZhuo Can you check latest review comments? I think it's getting close to merge and I have a follow-up PR #876 that depends on this.

manuzhang · 2026-09-04T01:25:35Z

+  ///
+  /// The caller owns the returned stream and must invoke its `release` callback
+  /// when the stream is no longer needed.
+  virtual Result<ArrowArrayStream> Scan() = 0;


It would be better to decouple with Arrow types.

Agreed. Decoupling the metadata table API from Arrow C types would be cleaner. This likely deserves a metadata-specific batch reader abstraction rather than reusing the file-oriented Reader interface. Since that is a broader API change, I would prefer to address it in a follow-up PR and keep ArrowArrayStream for this PR.

Well, this defines the basic interface for all metadata tables. I think it will be better to get it right first, maybe in a separate PR. @wgtmac What do you think?

WZhuo · 2026-09-08T03:05:42Z

@WZhuo Can you check latest review comments? I think it's getting close to merge and I have a follow-up PR #876 that depends on this.

Ok, I'll fix the comments

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

+  const bool has_summary = !snapshot.summary.empty();
+  auto summary = snapshot.summary;
+  summary.erase(SnapshotSummaryFields::kOperation);
+  if (!has_summary) {
+    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));
+  } else {
+    ICEBERG_RETURN_UNEXPECTED(AppendStringMap(builder.column(5), summary));


+  const bool has_summary = !snapshot.summary.empty();
+  auto summary = snapshot.summary;
+  summary.erase(SnapshotSummaryFields::kOperation);
+  if (!has_summary) {
+    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));
+  } else {
+    ICEBERG_RETURN_UNEXPECTED(AppendStringMap(builder.column(5), summary));


+  const bool has_summary = !snapshot.summary.empty();
+  auto summary = snapshot.summary;
+  summary.erase(SnapshotSummaryFields::kOperation);
+  if (!has_summary) {
+    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));
+  } else {
+    ICEBERG_RETURN_UNEXPECTED(AppendStringMap(builder.column(5), summary));
+  }


+TEST_F(SnapshotsTableTest, ScanReturnsMultipleBatches) {
+  auto snapshot = MakeTestSnapshots().first;
+  std::vector<std::shared_ptr<Snapshot>> snapshots(1025, snapshot);
+  ICEBERG_UNWRAP_OR_FAIL(auto table, MakeTableWithSnapshots(std::move(snapshots),
+                                                            /*current_snapshot_id=*/1));
+  ICEBERG_UNWRAP_OR_FAIL(auto snapshots_table,
+                         MetadataTable::Make<SnapshotsTable>(table));
+
+  ICEBERG_UNWRAP_OR_FAIL(auto stream, snapshots_table->Scan());
+  ICEBERG_UNWRAP_OR_FAIL(auto batches, ReadAllBatches(std::move(stream)));
+  ASSERT_EQ(batches.size(), 2);
+  EXPECT_EQ(batches[0]->num_rows(), 1024);
+  EXPECT_EQ(batches[1]->num_rows(), 1);
+}


Copilot

🟡 Changes recommended

The snapshots summary filtering behavior and its test expectations conflict with the PR’s stated contract (null vs empty summary after removing operation).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 15/15 changed files
Comments generated: 2
Review effort level: Lite

+  const bool has_summary = !snapshot.summary.empty();
+  auto summary = snapshot.summary;
+  summary.erase(SnapshotSummaryFields::kOperation);
+  if (!has_summary) {
+    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));
+  } else {
+    ICEBERG_RETURN_UNEXPECTED(AppendStringMap(builder.column(5), summary));
+  }


+  EXPECT_TRUE(summaries->IsNull(0));
+  EXPECT_FALSE(summaries->IsNull(1));
+  EXPECT_EQ(summaries->value_length(1), 0);


- Add Scan() virtual method and Scan() convenience overload to MetadataTable - Add SnapshotSelection struct for time-travel snapshot resolution - Add supports_time_travel() concrete method driven by kind() - Implement SnapshotsTable::Scan() to materialize snapshot rows via ArrowRowBuilder

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

The snapshots summary null-vs-empty-map behavior is inconsistent with the PR description (and should be aligned in code/tests before approval).

Review details

Suppressed comments (1)

src/iceberg/inspect/snapshots_table.cc:76

AppendSnapshot() decides whether to emit a null summary value based on snapshot.summary.empty() before removing SnapshotSummaryFields::kOperation. This means a snapshot whose summary only contains operation will currently emit a non-null but empty map, which contradicts the PR description (“emit null when the remaining summary is empty”). Consider checking summary.empty() after erasing kOperation and emitting null when the filtered map is empty (and update SnapshotsTableTest.ScanEmptySummary accordingly).

  const bool has_summary = !snapshot.summary.empty();
  auto summary = snapshot.summary;
  summary.erase(SnapshotSummaryFields::kOperation);
  if (!has_summary) {
    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));
  } else {
    ICEBERG_RETURN_UNEXPECTED(AppendStringMap(builder.column(5), summary));
  }

Files reviewed: 15/15 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot

🔵 Needs a closer look

The snapshots summary null/empty emission behavior in SnapshotsTable is inconsistent with the PR description and should be reconciled (code/tests/docs) before approval.

Review details

Suppressed comments (1)

src/iceberg/inspect/snapshots_table.cc:72

The summary null/empty handling doesn’t match the PR description (“emit null when the remaining summary is empty”). Here has_summary is computed before removing SnapshotSummaryFields::kOperation, so a summary that only contains operation becomes an empty map but is still emitted as a non-null (empty) map. Decide on the intended semantics (null vs empty map after filtering) and make code + tests/description consistent (typically: erase operation first, then if the filtered map is empty append null).

  const bool has_summary = !snapshot.summary.empty();
  auto summary = snapshot.summary;
  summary.erase(SnapshotSummaryFields::kOperation);
  if (!has_summary) {
    ICEBERG_RETURN_UNEXPECTED(AppendNull(builder.column(5)));

Files reviewed: 14/14 changed files
Comments generated: 0 new
Review effort level: Lite

WZhuo marked this pull request as ready for review July 2, 2026 08:46

WZhuo force-pushed the inspect branch 3 times, most recently from 28c4513 to a53f8ce Compare July 6, 2026 03:37

WZhuo changed the title ~~feat(inspect): expand MetadataTable framework for all 16 metadata table types~~ feat(inspect): implement SnapshotsTable scanning Jul 6, 2026

WZhuo force-pushed the inspect branch 3 times, most recently from 9bc23e1 to 04b657f Compare July 7, 2026 02:04

WZhuo force-pushed the inspect branch 2 times, most recently from efc76b8 to 3b11911 Compare July 17, 2026 02:19

manuzhang added this to the 0.4.0 milestone Jul 21, 2026

manuzhang requested a lite review from Copilot July 21, 2026 03:39

Copilot started reviewing on behalf of manuzhang July 21, 2026 03:42 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread src/iceberg/inspect/metadata_table.h Outdated

Comment thread src/iceberg/inspect/metadata_table.cc Outdated

Comment thread src/iceberg/test/metadata_table_test_base.h Outdated

Comment thread src/iceberg/inspect/metadata_table.h Outdated

Copilot AI review requested due to automatic review settings July 22, 2026 05:32

Copilot started reviewing on behalf of WZhuo July 22, 2026 05:33 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread src/iceberg/test/snapshots_table_test.cc

Comment thread src/iceberg/test/snapshots_table_test.cc Outdated

Copilot AI review requested due to automatic review settings July 23, 2026 01:44

Copilot started reviewing on behalf of WZhuo July 23, 2026 01:45 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread src/iceberg/test/metadata_table_test_base.h Outdated

Copilot AI review requested due to automatic review settings July 24, 2026 01:38

Copilot started reviewing on behalf of WZhuo July 24, 2026 01:38 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread src/iceberg/test/metadata_table_test_base.h Outdated

Comment thread src/iceberg/test/metadata_table_test.cc Outdated

Comment thread src/iceberg/test/snapshots_table_test.cc Outdated

Copilot AI review requested due to automatic review settings July 24, 2026 01:43

Copilot started reviewing on behalf of WZhuo July 24, 2026 01:44 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread src/iceberg/inspect/snapshots_table.cc Outdated

Comment thread src/iceberg/test/metadata_table_test_base.h Outdated

Copilot AI review requested due to automatic review settings July 27, 2026 02:05

Copilot started reviewing on behalf of WZhuo July 27, 2026 02:05 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread src/iceberg/inspect/snapshots_table.cc Outdated

wgtmac reviewed Jul 29, 2026

View reviewed changes

Comment thread src/iceberg/inspect/metadata_table.h Outdated

Comment thread src/iceberg/inspect/metadata_table.h Outdated

Comment thread src/iceberg/inspect/metadata_table.h Outdated

Comment thread src/iceberg/inspect/metadata_table.h Outdated

manuzhang reviewed Sep 4, 2026

View reviewed changes

WZhuo requested a review from manuzhang September 8, 2026 02:53

Copilot AI review requested due to automatic review settings September 8, 2026 12:44

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Copilot started reviewing on behalf of WZhuo September 8, 2026 13:48 View session

Copilot AI review requested due to automatic review settings September 10, 2026 06:48

Copilot started reviewing on behalf of WZhuo September 10, 2026 06:50 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

WZhuo and others added 9 commits September 14, 2026 09:32

fix(inspect): address metadata table review feedback

222ba69

test(inspect): address snapshots table review feedback

f84e14a

Potential fix for pull request finding

2be02cd

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fix formatting issues in metadata_table_test_base.h

23a57ea

fix(inspect): address remaining review feedback

7e4e87e

feat(arrow): expose row count from ArrowRowBuilder

7e48608

refactor(inspect): refine metadata table scan APIs

6c6086d

fix(inspect): address metadata table review feedback

9bb29f6

Copilot AI review requested due to automatic review settings September 14, 2026 01:35

WZhuo force-pushed the inspect branch from d315d11 to 294c570 Compare September 14, 2026 01:35

Copilot started reviewing on behalf of WZhuo September 14, 2026 01:36 View session

Copilot AI reviewed Sep 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 14, 2026 01:49

WZhuo force-pushed the inspect branch from 294c570 to 323e904 Compare September 14, 2026 01:49

Copilot started reviewing on behalf of WZhuo September 14, 2026 01:49 View session

Copilot AI reviewed Sep 14, 2026

View reviewed changes

fix(inspect): address remaining PR review comments

09ca619

Copilot AI review requested due to automatic review settings September 14, 2026 02:15

WZhuo force-pushed the inspect branch from 323e904 to 09ca619 Compare September 14, 2026 02:15

Uh oh!

Conversation

WZhuo commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Metadata table APIs

Snapshots table

Arrow row builder

Tests

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manuzhang commented Sep 3, 2026

Uh oh!

manuzhang Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

WZhuo Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

manuzhang Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WZhuo commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

WZhuo commented Jul 2, 2026 •

edited

Loading

manuzhang Sep 8, 2026 •

edited

Loading