Skip to content

feat(inspect): add files, partitions, and manifests metadata tables - #876

Draft
manuzhang wants to merge 8 commits into
apache:mainfrom
manuzhang:agent/add-system-metadata-tables
Draft

manuzhang wants to merge 8 commits into
apache:mainfrom
manuzhang:agent/add-system-metadata-tables

Conversation

@manuzhang

@manuzhang manuzhang commented Aug 6, 2026 •

Copy link
Copy Markdown
Member

Add FilesTable, PartitionsTable, and ManifestsTable through Table::Inspect(), using metadata scan builders and task readers. Scans capture their metadata, schema, projection, and FileIO so planned tasks and streams survive source refreshes and owner destruction. Snapshot selection supports IDs, timestamps, and named references; non-main references cannot be combined with an ID or timestamp. Tasks support whole-column selection and projection, including reordered and empty projections, and configurable Arrow batch sizes.

Both NewScan() and MetadataScanBuilder::Make() derive schemas from captured metadata. Files and partitions tables support concurrent builder creation with mutex-protected schema caches; MetadataTable::schema() returns a shared pointer by value so callers retain ownership while the cache changes. Existing history, snapshots, and refs tables use the same ownership contract.

File scans read manifest entries on demand. Partition aggregation projects across evolved specs and canonicalizes NaNs while preserving signed zeros. Manifest rows expose file counts and readable partition bounds, recovering dropped source-column types from retained schema history, including identity and truncate transforms and numeric promotion. Partition types are cached per spec within a scan.

Validation: all 125 tests in metadata_table_test and cpp20_compatibility_test passed in a Debug build with Arrow/Parquet 23.0.1 (PyArrow libraries) and C++20 compatibility enabled. The portability follow-up uses mutex-protected caches for Apple libc++ and the project error-return alias for C++20. pre-commit, git diff --check, and range-diff review passed.

@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from f4e4c83 to b0f596c Compare August 17, 2026 10:52
Copilot AI lite review requested due to automatic review settings August 17, 2026 10:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new “system” metadata tables under iceberg/inspect/ and shared utilities to scan Iceberg metadata into bounded Arrow batches, aligning C++ inspect APIs with Iceberg’s reference behavior. This builds on the streaming metadata-table API work referenced in #801.

Changes:

  • Adds new metadata table implementations: branches, tags, files, partitions, manifests (plus supporting shared stream/util code).
  • Refactors metadata-table base APIs to support typed factories, bounded batch sizing, and time-travel scans via SnapshotSelection.
  • Adds/updates unit tests and wires them into CMake/Meson builds.

Reviewed changes

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

Show a summary per file
File Description
src/iceberg/type_fwd.h Forward declares the new inspect table classes.
src/iceberg/inspect/metadata_table.h Expands/modernizes the metadata-table base API (kinds, typed factory, batch size, time travel interface).
src/iceberg/inspect/metadata_table.cc Implements the refactored base behavior and TimeTravelMetadataTable scan dispatch.
src/iceberg/inspect/metadata_table_stream_internal.h Adds a shared Arrow stream implementation for row-vector-backed metadata tables.
src/iceberg/inspect/metadata_table_util_internal.h Declares shared internal helpers for snapshot resolution, schema building, partition projection, and live-file loading.
src/iceberg/inspect/metadata_table_util_internal.cc Implements shared internal utilities used by files/partitions/manifests scans.
src/iceberg/inspect/branches_table.h Declares the branches metadata table.
src/iceberg/inspect/branches_table.cc Implements branch reference scanning into Arrow batches.
src/iceberg/inspect/tags_table.h Declares the tags metadata table.
src/iceberg/inspect/tags_table.cc Implements tag reference scanning into Arrow batches.
src/iceberg/inspect/files_table.h Declares the snapshot-scoped files metadata table (time travel capable).
src/iceberg/inspect/files_table.cc Implements live-file scanning for a selected snapshot into Arrow batches.
src/iceberg/inspect/partitions_table.h Declares the snapshot-scoped partitions aggregates table (time travel capable).
src/iceberg/inspect/partitions_table.cc Implements partition aggregation over live files for a selected snapshot.
src/iceberg/inspect/manifests_table.h Declares the snapshot-scoped manifests table (time travel capable).
src/iceberg/inspect/manifests_table.cc Implements manifest-list scanning for a selected snapshot, including partition summaries.
src/iceberg/inspect/snapshots_table.h Extends snapshots table API with schema + streaming Scan().
src/iceberg/inspect/snapshots_table.cc Implements snapshots streaming scan with bounded batch size and resource cleanup.
src/iceberg/inspect/history_table.h Updates history table interface to match new base API expectations.
src/iceberg/inspect/history_table.cc Updates history table implementation; currently returns NotSupported for Scan().
src/iceberg/inspect/meson.build Installs the new public inspect headers.
src/iceberg/arrow_row_builder_internal.h Adds num_rows() to support batch-size enforcement in streaming producers.
src/iceberg/arrow_row_builder.cc Implements ArrowRowBuilder::num_rows().
src/iceberg/test/arrow_row_builder_test.cc Adds assertions covering the new num_rows() behavior.
src/iceberg/test/metadata_table_test_base.h Introduces a shared fixture/helpers for metadata-table tests.
src/iceberg/test/metadata_table_test.cc Updates tests to the typed metadata-table factory and validates time-travel support flags.
src/iceberg/test/history_table_test.cc Adds schema-focused unit test coverage for HistoryTable.
src/iceberg/test/snapshots_table_test.cc Adds snapshots streaming scan tests (schema/values/batching/null handling).
src/iceberg/test/system_metadata_tables_test.cc Adds integration-style tests for the new system metadata tables (refs/files/partitions/manifests and snapshot selection).
src/iceberg/test/CMakeLists.txt Wires the new/updated metadata-table tests into the CMake test target.
src/iceberg/test/meson.build Wires (some) of the new tests into Meson’s table_test target.
src/iceberg/CMakeLists.txt Adds new inspect sources to the main CMake build.
src/iceberg/meson.build Adds new inspect sources to the main Meson build.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/iceberg/test/meson.build Resolved
Comment thread src/iceberg/inspect/history_table.cc Resolved
Copilot AI review requested due to automatic review settings September 16, 2026 11:13
@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from b0f596c to cf28ec0 Compare September 16, 2026 11:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings affect decimal handling, schema correctness, manifest counts, and streaming memory bounds.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

src/iceberg/inspect/files_table.cc:54

  • These schema/type objects are fixed when FilesTable::Make runs, but ScanSnapshot resolves snapshots and loads files from the source table's current metadata. After source_table()->Refresh() with schema or partition evolution, the scan mixes new files/specs with the stale output schema, silently omitting new metrics or partition fields (or projecting them with the wrong type). Rebuild the scan schema from the same metadata snapshot used for the scan, or otherwise make the cached schema and source metadata consistent.
  ICEBERG_ASSIGN_OR_RAISE(auto partition_type, internal::UnifiedPartitionType(*table));
  ICEBERG_ASSIGN_OR_RAISE(auto schema,
                          internal::FilesTableSchema(*table_schema, partition_type));
  return std::unique_ptr<FilesTable>(new FilesTable(std::move(table), std::move(schema),
                                                    std::move(table_schema),
                                                    std::move(partition_type)));

src/iceberg/inspect/manifests_table.cc:123

  • When a manifest count is absent, .value_or(0) converts Iceberg's unknown count into a definitive zero. ManifestFile documents a null count as “assumed to be non-zero”, so $manifests reports incorrect counts for older manifests; preserve null for the applicable content counts and use zero only for the opposite content type.
      AppendInt(builder.column(5), data ? manifest.added_files_count.value_or(0) : 0));
  ICEBERG_RETURN_UNEXPECTED(
      AppendInt(builder.column(6), data ? manifest.existing_files_count.value_or(0) : 0));

src/iceberg/inspect/metadata_table_util_internal.cc:409

  • Because specs are processed newest-to-oldest, existing is the newer void definition and partition_field is the older non-void definition. Replacing definition here makes the unified schema use the old field name (for example, old_bucket) instead of the latest name (new_name), so the added legacy-evolution test will fail and consumers see a schema that no longer matches the current spec. Keep the latest definition/name and only adopt the older transform's type for decoding legacy values.
        if (existing_transform == TransformType::kVoid &&
            current_transform != TransformType::kVoid) {
          iter->second.definition = &partition_field;
          iter->second.type = spec_field.type();
        }

src/iceberg/inspect/metadata_table_util_internal.cc:617

  • This vector accumulates every live file returned by every manifest, while LiveEntries() has already materialized each manifest, and the stream is created only after the full vector is built. For a large snapshot, the Arrow batch limit does not bound scan memory or delay the first batch until the entire snapshot has been read. Feed entries through a cursor/iterator directly into bounded batches (or otherwise avoid retaining the complete result).
  std::vector<LiveFile> files;
  • Files reviewed: 21/21 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/iceberg/inspect/files_table.cc Resolved
Copilot AI review requested due to automatic review settings September 16, 2026 11:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical refresh-staleness findings and additional schema, validation, and projection issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/iceberg/inspect/metadata_table_util_internal.cc:383

  • On an unpartitioned table this erases partition from the public files schema even though DataFile::Type still defines it as an empty struct. Consumers therefore get a different column set for partitioned versus unpartitioned tables; the corresponding conditional in PartitionsTable::Make also drops partition and spec_id. Keep these empty-struct/metadata columns and append empty partition values so the metadata-table schema remains stable.
  if (partition_type->fields().empty()) {
    std::erase_if(fields, [](const SchemaField& field) {
      return field.field_id() == DataFile::kPartitionFieldId;
    });
  }

src/iceberg/inspect/metadata_table_util_internal.cc:75

  • An unknown named reference is caller-provided selection input, but ICEBERG_CHECK maps this failure to ErrorKind::kValidationFailed. Scan(SnapshotSelection{.ref_name = ...}) therefore reports a state error instead of the invalid-argument error used for invalid selection values. Use ICEBERG_PRECHECK for this lookup, consistent with the null-reference check below.
  ICEBERG_CHECK(ref != metadata->refs.end(), "Cannot find snapshot reference '{}'",
                ref_name);

src/iceberg/inspect/partitions_table.cc:259

  • PartitionsTable only consumes the partition, content, record count, and file size from each data file, but this calls LiveFiles without a projection. ManifestReader::ReadEntries consequently materializes the complete data-file schema, including all stats maps, bounds, and key metadata, for every live entry before aggregation; wide metrics can make $partitions scans unnecessarily memory-heavy. Add a projection path for LiveFiles and select only the fields needed by partition projection and UpdateCounts, as Java does for this table.
  ICEBERG_ASSIGN_OR_RAISE(auto files, internal::LiveFiles(*source_table(), snapshot));
  • Files reviewed: 25/25 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/iceberg/inspect/files_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Copilot AI review requested due to automatic review settings September 17, 2026 03:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from 8fe47a4 to 5c8c962 Compare October 10, 2026 15:15
Copilot AI lite review requested due to automatic review settings October 10, 2026 15:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot AI lite review requested due to automatic review settings October 10, 2026 16:22
@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from 5c8c962 to 93234f2 Compare October 10, 2026 16:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/iceberg/inspect/partitions_table.cc Resolved
@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from 93234f2 to a0902e1 Compare October 10, 2026 16:49
Copilot AI lite review requested due to automatic review settings October 10, 2026 16:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Missing manifest partition summaries are emitted as empty lists instead of null, and the duplicate PR concern remains.

3 open findings
1 resolved since last review

🧠 Review effort: Lite

Copilot AI lite review requested due to automatic review settings October 13, 2026 09:24
@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from a0902e1 to 8556d19 Compare October 13, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/iceberg/inspect/files_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/refs_table.cc Resolved
Copilot AI lite review requested due to automatic review settings October 13, 2026 13:54
@manuzhang manuzhang changed the title feat(inspect): add system metadata tables feat(inspect): add files, partitions, and manifests metadata tables Oct 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/iceberg/inspect/manifests_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Copilot AI lite review requested due to automatic review settings October 13, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot review overview

18 open findings

🧠 Review effort: Lite

Comment thread src/iceberg/test/CMakeLists.txt Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
Comment thread src/iceberg/inspect/partitions_table.cc Resolved
@github-actions

Copy link
Copy Markdown

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions Bot added the stale label Nov 13, 2026
@manuzhang
manuzhang force-pushed the agent/add-system-metadata-tables branch from 10d3c38 to 3539c08 Compare November 13, 2026 09:40
Copilot AI lite review requested due to automatic review settings November 13, 2026 09:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI lite review requested due to automatic review settings November 13, 2026 10:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Use mutex-protected shared pointers where atomic<shared_ptr> is unavailable in Apple libc++. Preserve concurrent scan ownership and use the project unexpected alias for C++20-compatible error propagation.

Co-authored-by: Codex <codex@openai.com>
Copilot AI lite review requested due to automatic review settings November 13, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants