Skip to content

Core: add labels to the load table/view read path - #18045

Merged
laskoviymishka merged 5 commits into
mainfrom
laskoviymishka/labels-serde
Sep 17, 2026
Merged

laskoviymishka merged 5 commits into
mainfrom
laskoviymishka/labels-serde

Conversation

@laskoviymishka

@laskoviymishka laskoviymishka commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Part 1 of a 5-PR stack implementing the client/engine side of catalog labels. The REST spec was merged in #15750.

This PR adds the Java model and REST serde for the labels field returned on the load-table and load-view responses:

  • Labels and FieldLabel immutable value types
  • LabelsParser / FieldLabelParser (wire keys object-labels, fields, field-id)
  • wires labels into LoadTableResponse / LoadViewResponse and their parsers, alongside the existing remote-signing-config
  • unit and round-trip tests

Note: the Java type is FieldLabel (singular) while the OpenAPI schema stays FieldLabels — an intentional Java-vs-spec divergence so fieldLabel.labels() reads cleanly. The wire contract (field-id, fields, labels) is unchanged.

Stack (linear; each based on the previous)

  1. Core: add labels to the load table/view read path #18045 — read-path serde (this PR)
  2. Core: expose catalog labels on the loaded table via SupportsLabels #18046 — expose on the loaded table (SupportsLabels)
  3. Core: add labels metadata table #18048 — labels metadata table
  4. Spark 4.1: expose catalog labels via DESCRIBE #18049 — Spark 4.1 DESCRIBE
  5. OpenAPI: emit labels from the REST server test fixture #18047 — REST server test fixture

Restructures the earlier cross-fork PRs (#17337 / #17410 / #17411) into a reviewable native stack; those remain open for now.

@nastra
nastra requested a review from danielcweeks September 11, 2026 13:57
Reference client implementation for the IRC labels read-path spec change
(#15750). Without it, RESTObjectMapper (which sets
FAIL_ON_UNKNOWN_PROPERTIES = false) silently drops the labels field on
deserialization, so labels returned by a catalog are invisible to the Java
client.

New rest.labels package (mirroring rest.credentials): Labels (object + fields
sub-scopes, with a shared empty instance) and FieldLabels (per-field, keyed by
field-id) value types as immutables interfaces, each with a JSON parser.
FieldLabels validates field-id >= 1 and a non-empty labels map, matching
Credential.

Wire an optional labels field into LoadTableResponse / LoadViewResponse and
their parsers. labels() never returns null (empty instance when absent), on
LoadViewResponse via a @Value.Default default method so no interface API break
is introduced. Labels are omitted from the wire when absent (or empty), so the
change is additive and backward compatible.
@laskoviymishka
laskoviymishka force-pushed the laskoviymishka/labels-serde branch from 98d20c7 to 4cf923b Compare September 14, 2026 15:31
laskoviymishka and others added 2 commits September 15, 2026 13:56
Address review nit: a single FieldLabel value was named `fieldLabels`
(plural). Rename the parameter in FieldLabelParser, the loop variable in
LabelsParser, and the test locals, and singularize the related
precondition messages.

Co-authored-by: Isaac <[email protected]>
The previous commit singularized the precondition messages in
FieldLabelParser, which broke TestFieldLabelParser.nullCheck (it asserts
the exact text). Keep the identifier rename fieldLabels -> fieldLabel but
restore the original messages ("Invalid field labels: null",
"Cannot parse field labels from null object").

Co-authored-by: Isaac <[email protected]>
@nastra

nastra commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

the changes themselves LGTM but the one concern I have is that we're now adding a REST-specific concept (Labels, FieldLabel) outside of the org.apache.iceberg.rest package, meaning that those things become part of the public API immediately.
@danielcweeks any thoughts on that?

@danielcweeks

Copy link
Copy Markdown
Contributor

the changes themselves LGTM but the one concern I have is that we're now adding a REST-specific concept (Labels, FieldLabel) outside of the org.apache.iceberg.rest package, meaning that those things become part of the public API immediately. @danielcweeks any thoughts on that?

@nastra this actually isn't REST-specific. For example, you can implement catalog labels with any catalog implementation (e.g. BigLake Catalog). While we expose this with REST, pretty much any implementation can support labels, so I think the packaging is correct.

- Make TestFieldLabelParser and TestLabelsParser (and their test methods)
  package private.
- Annotate Labels.isEmpty() with @Value.Derived, matching the pattern in
  RemoteSigningConfig.

Co-authored-by: Isaac <[email protected]>
Address review: default the builder's labels field to Labels.EMPTY, matching
remoteSigningConfig, so build() never passes null.

Co-authored-by: Isaac <[email protected]>

@pvary pvary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
Thanks @laskoviymishka!

@laskoviymishka
laskoviymishka merged commit f1c6206 into main Sep 17, 2026
42 checks passed
@laskoviymishka
laskoviymishka deleted the laskoviymishka/labels-serde branch September 17, 2026 13:01
NihalJain added a commit to NihalJain/iceberg-cpp that referenced this pull request Oct 12, 2026
Parse the optional `labels` field of the REST load-table response and
expose it through Table::labels(), mirroring apache/iceberg#18045 and
live on the Table instance instead of TableMetadata.

Part of apache#938.
NihalJain added a commit to NihalJain/iceberg-cpp that referenced this pull request Oct 12, 2026
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.
wgtmac pushed a commit to NihalJain/iceberg-cpp that referenced this pull request Oct 14, 2026
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.
wgtmac pushed a commit to apache/iceberg-cpp that referenced this pull request Oct 14, 2026
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.
Fokko pushed a commit to yayaha/iceberg-python that referenced this pull request Nov 10, 2026
…he#4320)

* Parse catalog labels in REST load table and load view responses

The REST spec adds an optional `labels` field to LoadTableResult and
LoadViewResult for catalog-provided enrichment such as ownership or
classification. PyIceberg dropped the field. Add `Labels` and
`FieldLabel` models and parse `labels` on `TableResponse` and
`ViewResponse`, matching the Java read path (apache/iceberg#18045).

* Read non-object labels as empty

A string, list, number, or bool `labels` value failed the whole load.
Java `LabelsParser` reads keys only under `hasNonNull`, so it returns
empty labels for any non-object value. Do the same.
Fokko added a commit to rahulsmahadev/iceberg-python that referenced this pull request Nov 11, 2026
* Parse catalog labels in REST load table and load view responses

The REST spec adds an optional `labels` field to LoadTableResult and
LoadViewResult for catalog-provided enrichment such as ownership or
classification. PyIceberg dropped the field. Add `Labels` and
`FieldLabel` models and parse `labels` on `TableResponse` and
`ViewResponse`, matching the Java read path (apache/iceberg#18045).

* Read non-object labels as empty

A string, list, number, or bool `labels` value failed the whole load.
Java `LabelsParser` reads keys only under `hasNonNull`, so it returns
empty labels for any non-object value. Do the same.

* Expose catalog labels on Table

Labels parsed from the load response were not reachable from the
loaded table. Add `Table.labels`. The REST catalog sets it on load,
create, and register. Other catalogs and staged tables
(`create_table_transaction`) get empty labels. Labels are not table
state: they are not part of equality, and refresh and commits keep
them. This matches the Java `SupportsLabels` behavior
(apache/iceberg#18046).

* Test labels on create, register, and pickle

Create and register hand labels to the same `_response_to_table` as
load, but only load was tested. Check all three. Also pin that a
pickled table keeps its labels, the one intended difference from Java,
where `BaseTable.labels` is transient.

---------

Co-authored-by: Fokko Driesprong <[email protected]>
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.

4 participants