feat: support catalog labels on tables loaded from REST catalogs by NihalJain · Pull Request #960 · apache/iceberg-cpp

NihalJain · 2026-09-23T18:58:03Z

Parse the optional labels field of the REST load-table response and expose it through Table::labels(), mirroring apache/iceberg#18045 and apache/iceberg#18046. Labels are catalog-provided rather than table state, so they live on the Table instance instead of TableMetadata.

Part of #938.

Unit tests cover the serde (all label shapes, null/absent handling, invalid input, strict int32 field-id) and the Table side (default empty, provided labels exposed, unchanged by Refresh()). The REST-response-to-Table wiring isn't reachable from unit tests, so I also checked it locally against the REST fixture behind a proxy that adds labels, covering create, load and register. Test code posted at https://gist.github.com/NihalJain/847d9137f090db63978a56c9fa84cd0b

Proper integration tests come in C3 once apache/iceberg#18047 ships in the fixture image.

AI Disclosure

Model: Opus 5.5
Platform/Tool: Claude Code
Human Oversight: Fully reviewed
Prompt Summary: Research how catalog labels were implemented in Iceberg Java, port the read path to iceberg-cpp for C1 of Catalog Labels: client-library read support for C++ #938 with matching semantics, verify it with unit tests and an end-to-end run against the REST fixture, and open a PR to apache/iceberg-cpp

Copilot

Copilot review overview

🔵 Needs a closer look

The staged create-table path must preserve catalog-provided labels.

Review effort: Lite
Findings: None

What changed in this PR

Adds REST catalog label parsing and exposes catalog-provided labels through Table::labels().

Changes:

Adds label models, serde support, and strict field-ID parsing.
Wires labels into REST table responses and table instances.
Adds serde and table behavior tests.

File	Summary
`src/iceberg/util/json_util_internal.h`	Adds strict int32 parsing.
`src/iceberg/type_fwd.h`	Adds label forward declarations.
`src/iceberg/test/table_test.cc`	Tests table label behavior.
`src/iceberg/test/rest_json_serde_test.cc`	Tests label serialization and parsing.
`src/iceberg/table.h`	Adds the labels API.
`src/iceberg/table.cc`	Stores and exposes labels.
`src/iceberg/labels.h`	Defines label models.
`src/iceberg/catalog/rest/types.h`	Adds labels to load results; staged table construction still drops them (moderate issue).
`src/iceberg/catalog/rest/types.cc`	Includes labels in equality checks.
`src/iceberg/catalog/rest/rest_catalog.cc`	Wires labels into REST table handling.
`src/iceberg/catalog/rest/json_serde.cc`	Implements label JSON serde.
`src/iceberg/catalog/rest/json_serde_internal.h`	Declares label serde functions.

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

wgtmac · 2026-10-11T14:34:34Z

Thanks for adding this! Could you please rebase and resolve merge conflicts?

NihalJain · 2026-10-12T07:05:38Z

Sure let me do that. Will update by EOD!

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.

Copilot review overview

1 open finding

LabelsFromJson calls json.find(...) without first verifying json is an object. For non-object… · New

🧠 Review effort: Lite

NihalJain · 2026-10-12T15:09:24Z

Rebased onto main. field-id parsing now uses GetJsonInteger<int32_t> from #940, and labels.h is added to the C++20 public-header list from #936. No other functional changes. Unit tests, pre-commit, and an end-to-end check against the REST fixture all verified locally.

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.

Copilot review overview

3 open findings

LabelsFromJson claims (and tests expect) that non-object "labels" values (e.g., string/array)… · New
LabelsFromJson calls json.find(...) without first verifying json is an object. For non-object…
Table::Make is exported (ICEBERG_EXPORT) and its mangled signature changes by adding the… · New

🧠 Review effort: Lite

Parse the optional `labels` field of the REST load-table response and expose it through Table::labels(), mirroring apache/iceberg#18045 and apache/iceberg#18046. Labels are catalog-provided rather than table state, so they live on the Table instance instead of TableMetadata. Part of apache#938.

Clarify label lifecycle and cover refresh, empty transactions, and REST load/create/register in both planning modes.

wgtmac

Thanks for adding this new feature! I rebased this onto main, reduced memory overhead for unlabeled tables, and improved the docs and test coverage.

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.

Copilot review overview

4 open findings

LabelsFromJson calls json.find(...) without first verifying json is an object. For non-object…
Same ABI concern as Table::Make: this modifies an exported factory function signature. If ABI… · New
This changes the signature (and mangled symbol) of an ICEBERG_EXPORT function. Even with a… · New
The comment says non-object labels should be treated as absent, but LabelsFromJson doesn’t… · New

2 resolved since last review

🧠 Review effort: Lite

Keep complete label definitions available for default unique_ptr arguments and remove redundant label getter tests.

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.

Copilot review overview

6 open findings

Adding a parameter to the exported Table::Make changes its mangled name and is an ABI break for… · New
LabelsFromJson calls json.find(...) without first verifying json is an object. For non-object…
Making the std::unique_ptr itself const prevents move construction/assignment of Table… · New
This changes the signature (and mangled symbol) of an ICEBERG_EXPORT function. Even with a…
Same ABI concern as Table::Make: this modifies an exported factory function signature. If ABI…
The comment says non-object labels should be treated as absent, but LabelsFromJson doesn’t…

🧠 Review effort: Lite

Copilot AI lite review requested due to automatic review settings September 23, 2026 18:58

Copilot started reviewing on behalf of NihalJain September 23, 2026 19:04

View session

NihalJain mentioned this pull request Sep 23, 2026

Catalog Labels: client-library read support for C++ #938

Open

4 tasks

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings October 12, 2026 14:50

NihalJain force-pushed the feat/catalog-labels branch 2 times, most recently from 37376c2 to dd1aee6 Compare October 12, 2026 14:55

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread src/iceberg/catalog/rest/json_serde.cc

Copilot AI lite review requested due to automatic review settings October 12, 2026 15:07

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Comment thread src/iceberg/catalog/rest/json_serde.cc

Comment thread src/iceberg/table.h Outdated

Copilot stopped reviewing on behalf of NihalJain due to an error October 12, 2026 16:20
The job was not acquired by a runner even after multiple attempts.

Copilot stopped reviewing on behalf of NihalJain due to an error October 12, 2026 16:34
The job was not acquired by a runner even after multiple attempts.

NihalJain and others added 2 commits October 14, 2026 16:13

refactor: allocate table labels only when present

b856413

Clarify label lifecycle and cover refresh, empty transactions, and REST load/create/register in both planning modes.

Copilot AI lite review requested due to automatic review settings October 14, 2026 09:46

wgtmac force-pushed the feat/catalog-labels branch from dd1aee6 to b856413 Compare October 14, 2026 09:46

wgtmac approved these changes Oct 14, 2026

View reviewed changes

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread src/iceberg/catalog/rest/rest_table.h Outdated

Comment thread src/iceberg/table.h Outdated

Comment thread src/iceberg/catalog/rest/json_serde.cc

Copilot stopped reviewing on behalf of NihalJain due to an error October 14, 2026 11:37
The job was not acquired by a runner even after multiple attempts.

refactor: pass table labels through unique ownership

4ef2834

Keep complete label definitions available for default unique_ptr arguments and remove redundant label getter tests.

Copilot AI lite review requested due to automatic review settings October 14, 2026 12:29

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread src/iceberg/table.h

Comment thread src/iceberg/table.h

Copilot stopped reviewing on behalf of NihalJain due to an error October 14, 2026 13:34
The job was not acquired by a runner even after multiple attempts.

wgtmac merged commit ad221b7 into apache:main Oct 14, 2026
15 checks passed

Conversation

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

wgtmac commented Oct 11, 2026

Uh oh!

NihalJain commented Oct 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

NihalJain commented Oct 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NihalJain commented Sep 23, 2026 •

edited

Loading