From b0b1a636f72371fae70aa4c4b17684ac0e9dc6b0 Mon Sep 17 00:00:00 2001 From: Nihal Jain Date: Thu, 24 Sep 2026 00:20:56 +0530 Subject: [PATCH] feat: support catalog labels on tables loaded from REST catalogs Parse the optional `labels` field of the REST load-table response and expose it through Table::labels(), mirroring apache/iceberg#18045 and #18046. Labels are catalog-provided rather than table state, so they live on the Table instance instead of TableMetadata. Part of #938. --- src/iceberg/catalog/rest/json_serde.cc | 66 ++++++ .../catalog/rest/json_serde_internal.h | 2 + src/iceberg/catalog/rest/rest_catalog.cc | 2 +- src/iceberg/catalog/rest/types.cc | 2 +- src/iceberg/catalog/rest/types.h | 3 + src/iceberg/labels.h | 56 +++++ src/iceberg/table.cc | 26 ++- src/iceberg/table.h | 14 +- src/iceberg/test/rest_json_serde_test.cc | 212 +++++++++++++++++- src/iceberg/test/table_test.cc | 50 +++++ src/iceberg/type_fwd.h | 2 + src/iceberg/util/json_util_internal.h | 23 ++ 12 files changed, 440 insertions(+), 18 deletions(-) create mode 100644 src/iceberg/labels.h diff --git a/src/iceberg/catalog/rest/json_serde.cc b/src/iceberg/catalog/rest/json_serde.cc index 3ce753f18..6f4cae94c 100644 --- a/src/iceberg/catalog/rest/json_serde.cc +++ b/src/iceberg/catalog/rest/json_serde.cc @@ -33,6 +33,7 @@ #include "iceberg/expression/json_serde_internal.h" #include "iceberg/file_format.h" #include "iceberg/json_serde_internal.h" +#include "iceberg/labels.h" #include "iceberg/manifest/manifest_entry.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" @@ -72,6 +73,10 @@ constexpr std::string_view kDestination = "destination"; constexpr std::string_view kMetadata = "metadata"; constexpr std::string_view kConfig = "config"; constexpr std::string_view kStorageCredentials = "storage-credentials"; +constexpr std::string_view kLabels = "labels"; +constexpr std::string_view kObjectLabels = "object-labels"; +constexpr std::string_view kFields = "fields"; +constexpr std::string_view kFieldId = "field-id"; constexpr std::string_view kPrefix = "prefix"; constexpr std::string_view kIdentifiers = "identifiers"; constexpr std::string_view kOverrides = "overrides"; @@ -712,6 +717,59 @@ Result RenameTableRequestFromJson(const nlohmann::json& json return request; } +nlohmann::json ToJson(const FieldLabel& field_label) { + nlohmann::json json; + json[kFieldId] = field_label.field_id; + json[kLabels] = field_label.labels; + return json; +} + +Result FieldLabelFromJson(const nlohmann::json& json) { + if (!json.is_object()) { + return JsonParseError("Cannot parse field label from non-object: {}", + SafeDumpJson(json)); + } + FieldLabel field_label; + ICEBERG_ASSIGN_OR_RAISE(field_label.field_id, GetJsonInt32Strict(json, kFieldId)); + ICEBERG_ASSIGN_OR_RAISE(field_label.labels, + GetJsonValue(json, kLabels)); + return field_label; +} + +nlohmann::json ToJson(const Labels& labels) { + nlohmann::json json = nlohmann::json::object(); + if (!labels.object_labels.empty()) { + json[kObjectLabels] = labels.object_labels; + } + if (!labels.fields.empty()) { + nlohmann::json fields = nlohmann::json::array(); + for (const auto& field_label : labels.fields) { + fields.push_back(ToJson(field_label)); + } + json[kFields] = std::move(fields); + } + return json; +} + +// A non-object value is treated as absent labels rather than failing the load. +Result LabelsFromJson(const nlohmann::json& json) { + Labels labels; + ICEBERG_ASSIGN_OR_RAISE( + labels.object_labels, + GetJsonValueOrDefault(json, kObjectLabels)); + if (auto it = json.find(kFields); it != json.end() && !it->is_null()) { + if (!it->is_array()) { + return JsonParseError("Cannot parse '{}' from non-array: {}", kFields, + SafeDumpJson(*it)); + } + for (const auto& entry : *it) { + ICEBERG_ASSIGN_OR_RAISE(auto field_label, FieldLabelFromJson(entry)); + labels.fields.push_back(std::move(field_label)); + } + } + return labels; +} + // LoadTableResult (used by CreateTableResponse, LoadTableResponse) Result ToJson(const LoadTableResult& result) { nlohmann::json json; @@ -726,6 +784,9 @@ Result ToJson(const LoadTableResult& result) { } json[kStorageCredentials] = std::move(creds); } + if (!result.labels.empty()) { + json[kLabels] = ToJson(result.labels); + } return json; } @@ -747,6 +808,9 @@ Result LoadTableResultFromJson(const nlohmann::json& json) { result.storage_credentials.push_back(std::move(cred)); } } + if (auto it = json.find(kLabels); it != json.end() && !it->is_null()) { + ICEBERG_ASSIGN_OR_RAISE(result.labels, LabelsFromJson(*it)); + } ICEBERG_RETURN_UNEXPECTED(result.Validate()); return result; } @@ -1216,5 +1280,7 @@ ICEBERG_DEFINE_FROM_JSON(CommitTableResponse) ICEBERG_DEFINE_FROM_JSON(OAuthTokenResponse) ICEBERG_DEFINE_FROM_JSON(PlanTableScanRequest) ICEBERG_DEFINE_FROM_JSON(FetchScanTasksRequest) +ICEBERG_DEFINE_FROM_JSON(FieldLabel) +ICEBERG_DEFINE_FROM_JSON(Labels) } // namespace iceberg::rest diff --git a/src/iceberg/catalog/rest/json_serde_internal.h b/src/iceberg/catalog/rest/json_serde_internal.h index 6e148e0d3..08a32143e 100644 --- a/src/iceberg/catalog/rest/json_serde_internal.h +++ b/src/iceberg/catalog/rest/json_serde_internal.h @@ -60,6 +60,8 @@ ICEBERG_DECLARE_JSON_SERDE(ListTablesResponse) ICEBERG_DECLARE_JSON_SERDE(RegisterTableRequest) ICEBERG_DECLARE_JSON_SERDE(RenameTableRequest) ICEBERG_DECLARE_JSON_SERDE(OAuthTokenResponse) +ICEBERG_DECLARE_JSON_SERDE(FieldLabel) +ICEBERG_DECLARE_JSON_SERDE(Labels) #undef ICEBERG_DECLARE_JSON_SERDE diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 9635aebef..04bddaea3 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -902,7 +902,7 @@ Result> RestCatalog::MakeTableFromLoadResult( return Table::Make(identifier, std::move(result.metadata), std::move(result.metadata_location), std::move(table_io), std::move(table_catalog), RestTableName(name_, identifier), - std::move(reporter)); + std::move(reporter), std::move(result.labels)); } Result> RestCatalog::MakeTableFromCommitResponse( diff --git a/src/iceberg/catalog/rest/types.cc b/src/iceberg/catalog/rest/types.cc index 84fba9a7c..c4c4f0f31 100644 --- a/src/iceberg/catalog/rest/types.cc +++ b/src/iceberg/catalog/rest/types.cc @@ -87,7 +87,7 @@ bool CreateTableRequest::operator==(const CreateTableRequest& other) const { bool LoadTableResult::operator==(const LoadTableResult& other) const { if (metadata_location != other.metadata_location || config != other.config || - storage_credentials != other.storage_credentials) { + storage_credentials != other.storage_credentials || labels != other.labels) { return false; } diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 20a59fa59..d7e0c0523 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -29,6 +29,7 @@ #include "iceberg/catalog/rest/endpoint.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/labels.h" #include "iceberg/result.h" #include "iceberg/storage_credential.h" #include "iceberg/table_identifier.h" @@ -188,6 +189,8 @@ struct ICEBERG_REST_EXPORT LoadTableResult { std::unordered_map config; /// \brief Vended storage credentials, one per URI prefix; empty if none. std::vector storage_credentials; + /// \brief Catalog-provided labels; empty if none. + Labels labels; /// \brief Validates the LoadTableResult. Status Validate() const { diff --git a/src/iceberg/labels.h b/src/iceberg/labels.h new file mode 100644 index 000000000..f97975b38 --- /dev/null +++ b/src/iceberg/labels.h @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/labels.h +/// \brief Catalog-provided labels for tables and their fields. + +#include +#include +#include +#include + +#include "iceberg/iceberg_export.h" + +namespace iceberg { + +/// \brief Catalog-provided labels for a single field, identified by its field ID. +struct ICEBERG_EXPORT FieldLabel { + int32_t field_id = 0; + std::unordered_map labels; + + bool operator==(const FieldLabel&) const = default; +}; + +/// \brief Optional catalog-provided labels returned when a table is loaded. +/// +/// Labels are advisory enrichment, not table state: they are not persisted with the +/// table and may be absent. +struct ICEBERG_EXPORT Labels { + std::unordered_map object_labels; + std::vector fields; + + /// \brief Returns true when there are neither object-level nor field-level labels. + bool empty() const { return object_labels.empty() && fields.empty(); } + + bool operator==(const Labels&) const = default; +}; + +} // namespace iceberg diff --git a/src/iceberg/table.cc b/src/iceberg/table.cc index c512c11df..1beb0ef8b 100644 --- a/src/iceberg/table.cc +++ b/src/iceberg/table.cc @@ -53,13 +53,11 @@ namespace iceberg { -Result> Table::Make(TableIdentifier identifier, - std::shared_ptr metadata, - std::string metadata_location, - std::shared_ptr io, - std::shared_ptr catalog, - std::string full_name, - std::shared_ptr reporter) { +Result> Table::Make( + TableIdentifier identifier, std::shared_ptr metadata, + std::string metadata_location, std::shared_ptr io, + std::shared_ptr catalog, std::string full_name, + std::shared_ptr reporter, Labels labels) { if (metadata == nullptr) [[unlikely]] { return InvalidArgument("Metadata cannot be null"); } @@ -72,9 +70,10 @@ Result> Table::Make(TableIdentifier identifier, if (catalog == nullptr) [[unlikely]] { return InvalidArgument("Catalog cannot be null"); } - return std::shared_ptr(new Table( - std::move(identifier), std::move(metadata), std::move(metadata_location), - std::move(io), std::move(catalog), std::move(full_name), std::move(reporter))); + return std::shared_ptr
(new Table(std::move(identifier), std::move(metadata), + std::move(metadata_location), std::move(io), + std::move(catalog), std::move(full_name), + std::move(reporter), std::move(labels))); } Table::~Table() = default; @@ -82,7 +81,7 @@ Table::~Table() = default; Table::Table(TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, std::shared_ptr catalog, std::string full_name, - std::shared_ptr reporter) + std::shared_ptr reporter, Labels labels) : identifier_(std::move(identifier)), full_name_(full_name.empty() ? identifier_.ToString() : std::move(full_name)), metadata_(std::move(metadata)), @@ -90,7 +89,8 @@ Table::Table(TableIdentifier identifier, std::shared_ptr metadata io_(std::move(io)), catalog_(std::move(catalog)), reporter_(std::move(reporter)), - metadata_cache_(std::make_unique(metadata_.get())) {} + metadata_cache_(std::make_unique(metadata_.get())), + labels_(std::move(labels)) {} const std::string& Table::uuid() const { return metadata_->table_uuid; } @@ -164,6 +164,8 @@ const std::shared_ptr& Table::catalog() const { return catalog_; } const std::shared_ptr& Table::reporter() const { return reporter_; } +const Labels& Table::labels() const { return labels_; } + Result> Table::location_provider() const { return LocationProvider::Make(metadata_->location, metadata_->properties); } diff --git a/src/iceberg/table.h b/src/iceberg/table.h index 4c0470cb9..b34ed80ad 100644 --- a/src/iceberg/table.h +++ b/src/iceberg/table.h @@ -29,6 +29,7 @@ #include #include "iceberg/iceberg_export.h" +#include "iceberg/labels.h" #include "iceberg/snapshot.h" #include "iceberg/table_identifier.h" #include "iceberg/type_fwd.h" @@ -50,11 +51,12 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ /// string representation of identifier when empty. /// \param[in] reporter Optional metrics reporter for this table. Defaults to nullptr /// (noop). + /// \param[in] labels Catalog-provided labels for this table. Defaults to empty. static Result> Make( TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, std::shared_ptr catalog, std::string full_name = "", - std::shared_ptr reporter = nullptr); + std::shared_ptr reporter = nullptr, Labels labels = {}); virtual ~Table(); @@ -130,6 +132,13 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ /// \brief Returns the metrics reporter for this table. const std::shared_ptr& reporter() const; + /// \brief Returns the catalog-provided labels for this table. + /// + /// Labels come only from the catalog's load, create or register response and are fixed + /// for the life of this table: Refresh() does not update them, and a table returned by + /// Transaction::Commit() has none. Load the table again to get current labels. + const Labels& labels() const; + /// \brief Returns a LocationProvider for this table Result> location_provider() const; @@ -216,7 +225,7 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ Table(TableIdentifier identifier, std::shared_ptr metadata, std::string metadata_location, std::shared_ptr io, std::shared_ptr catalog, std::string full_name, - std::shared_ptr reporter = nullptr); + std::shared_ptr reporter = nullptr, Labels labels = {}); const TableIdentifier identifier_; const std::string full_name_; @@ -226,6 +235,7 @@ class ICEBERG_EXPORT Table : public std::enable_shared_from_this
{ std::shared_ptr catalog_; std::shared_ptr reporter_; std::unique_ptr metadata_cache_; + const Labels labels_; }; /// \brief A table created by stage-create and not yet committed. diff --git a/src/iceberg/test/rest_json_serde_test.cc b/src/iceberg/test/rest_json_serde_test.cc index ec41e4a66..46bec4442 100644 --- a/src/iceberg/test/rest_json_serde_test.cc +++ b/src/iceberg/test/rest_json_serde_test.cc @@ -30,6 +30,7 @@ #include "iceberg/catalog/rest/types.h" #include "iceberg/expression/expressions.h" #include "iceberg/file_format.h" +#include "iceberg/labels.h" #include "iceberg/manifest/manifest_entry.h" #include "iceberg/partition_spec.h" #include "iceberg/result.h" @@ -82,6 +83,11 @@ static std::shared_ptr MakeSimpleTableMetadata() { }); } +std::string LoadTableJsonWithLabels(std::string_view labels) { + return std::string(R"({"labels":)") + std::string(labels) + + R"(,"metadata":{"format-version":2,"table-uuid":"test-uuid-1234","location":"s3://bucket/test","last-sequence-number":0,"last-updated-ms":0,"last-column-id":1,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0,"properties":{}}})"; +} + std::string LoadTableJsonWithCredentials(std::string_view storage_credentials) { return std::string(R"({"storage-credentials":)") + std::string(storage_credentials) + R"(,"metadata":{"format-version":2,"table-uuid":"test","location":"s3://test","last-sequence-number":0,"last-column-id":1,"last-updated-ms":0,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0}})"; @@ -1133,7 +1139,15 @@ INSTANTIATE_TEST_SUITE_P( .config = {{"s3.access-key-id", "AKIAtest"}, {"s3.secret-access-key", "secret"}, {"s3.session-token", "token"}, - {"client.region", "us-east-1"}}}}}}), + {"client.region", "us-east-1"}}}}}}, + LoadTableResultParam{ + .test_name = "WithLabels", + .expected_json_str = + R"({"labels":{"fields":[{"field-id":1,"labels":{"classification":"pii"}}],"object-labels":{"owner":"team-a"}},"metadata":{"current-schema-id":1,"current-snapshot-id":null,"default-sort-order-id":0,"default-spec-id":0,"format-version":2,"last-column-id":1,"last-partition-id":0,"last-sequence-number":0,"last-updated-ms":0,"location":"s3://bucket/test","metadata-log":[],"partition-specs":[{"fields":[],"spec-id":0}],"partition-statistics":[],"properties":{},"refs":{},"schemas":[{"fields":[{"id":1,"name":"id","required":true,"type":"int"}],"schema-id":1,"type":"struct"}],"snapshot-log":[],"snapshots":[],"sort-orders":[{"fields":[],"order-id":0}],"statistics":[],"table-uuid":"test-uuid-1234"}})", + .model = {.metadata = MakeSimpleTableMetadata(), + .labels = {.object_labels = {{"owner", "team-a"}}, + .fields = {{.field_id = 1, + .labels = {{"classification", "pii"}}}}}}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -1162,7 +1176,33 @@ INSTANTIATE_TEST_SUITE_P( .json_str = R"({"metadata":{"format-version":2,"table-uuid":"test-uuid-1234","location":"s3://bucket/test","last-sequence-number":0,"last-updated-ms":0,"last-column-id":1,"schemas":[{"type":"struct","schema-id":1,"fields":[{"id":1,"name":"id","type":"int","required":true}]}],"current-schema-id":1,"partition-specs":[{"spec-id":0,"fields":[]}],"default-spec-id":0,"last-partition-id":0,"sort-orders":[{"order-id":0,"fields":[]}],"default-sort-order-id":0,"properties":{}},"config":{"warehouse":"s3://bucket/warehouse"}})", .expected_model = {.metadata = MakeSimpleTableMetadata(), - .config = {{"warehouse", "s3://bucket/warehouse"}}}}), + .config = {{"warehouse", "s3://bucket/warehouse"}}}}, + LoadTableResultDeserializeParam{ + .test_name = "WithLabels", + .json_str = LoadTableJsonWithLabels( + R"({"object-labels":{"owner":"team-a"},"fields":[{"field-id":1,"labels":{"classification":"pii"}}]})"), + .expected_model = {.metadata = MakeSimpleTableMetadata(), + .labels = {.object_labels = {{"owner", "team-a"}}, + .fields = {{.field_id = 1, + .labels = {{"classification", + "pii"}}}}}}}, + LoadTableResultDeserializeParam{ + .test_name = "NullLabels", + .json_str = LoadTableJsonWithLabels("null"), + .expected_model = {.metadata = MakeSimpleTableMetadata()}}, + LoadTableResultDeserializeParam{ + .test_name = "EmptyLabels", + .json_str = LoadTableJsonWithLabels("{}"), + .expected_model = {.metadata = MakeSimpleTableMetadata()}}, + // Non-object labels are ignored rather than rejected + LoadTableResultDeserializeParam{ + .test_name = "StringLabels", + .json_str = LoadTableJsonWithLabels(R"("oops")"), + .expected_model = {.metadata = MakeSimpleTableMetadata()}}, + LoadTableResultDeserializeParam{ + .test_name = "ArrayLabels", + .json_str = LoadTableJsonWithLabels(R"([{"object-labels":{"k":"v"}}])"), + .expected_model = {.metadata = MakeSimpleTableMetadata()}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -1202,6 +1242,10 @@ INSTANTIATE_TEST_SUITE_P( .test_name = "InvalidMetadataContent", .invalid_json_str = R"({"metadata":{"format-version":"invalid"}})", .expected_error_message = "type must be number, but is string"}, + LoadTableResultInvalidParam{ + .test_name = "LabelsFieldsNotArray", + .invalid_json_str = LoadTableJsonWithLabels(R"({"fields":"oops"})"), + .expected_error_message = "Cannot parse 'fields' from non-array"}, LoadTableResultInvalidParam{ .test_name = "CredentialsNotArray", .invalid_json_str = LoadTableJsonWithCredentials(R"("oops")"), @@ -2632,4 +2676,168 @@ TEST(FetchPlanningResultResponseRoundtripTest, FailedWithError) { EXPECT_EQ(*result, *result2); } +DECLARE_ROUNDTRIP_TEST(FieldLabel) + +INSTANTIATE_TEST_SUITE_P( + FieldLabelCases, FieldLabelTest, + ::testing::Values( + FieldLabelParam{ + .test_name = "WithLabels", + .expected_json_str = + R"({"field-id":1,"labels":{"classification":"pii","owner":"team-a"}})", + .model = {.field_id = 1, + .labels = {{"classification", "pii"}, {"owner", "team-a"}}}}, + // A field label always writes "labels", even when empty + FieldLabelParam{.test_name = "EmptyLabels", + .expected_json_str = R"({"field-id":1,"labels":{}})", + .model = {.field_id = 1}}, + FieldLabelParam{.test_name = "MaxInt32FieldId", + .expected_json_str = R"({"field-id":2147483647,"labels":{}})", + .model = {.field_id = 2147483647}}, + FieldLabelParam{.test_name = "NegativeFieldId", + .expected_json_str = R"({"field-id":-1,"labels":{"k":"v"}})", + .model = {.field_id = -1, .labels = {{"k", "v"}}}}), + [](const ::testing::TestParamInfo& info) { + return info.param.test_name; + }); + +DECLARE_INVALID_TEST(FieldLabel) + +INSTANTIATE_TEST_SUITE_P( + FieldLabelInvalidCases, FieldLabelInvalidTest, + ::testing::Values( + FieldLabelInvalidParam{ + .test_name = "NotObject", + .invalid_json_str = R"([])", + .expected_error_message = "Cannot parse field label from non-object"}, + FieldLabelInvalidParam{.test_name = "MissingFieldId", + .invalid_json_str = R"({"labels":{}})", + .expected_error_message = "Missing 'field-id'"}, + FieldLabelInvalidParam{.test_name = "NullFieldId", + .invalid_json_str = R"({"field-id":null,"labels":{}})", + .expected_error_message = "Missing 'field-id'"}, + FieldLabelInvalidParam{ + .test_name = "FloatFieldId", + .invalid_json_str = R"({"field-id":1.5,"labels":{}})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}, + FieldLabelInvalidParam{ + .test_name = "BoolFieldId", + .invalid_json_str = R"({"field-id":true,"labels":{}})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}, + FieldLabelInvalidParam{ + .test_name = "StringFieldId", + .invalid_json_str = R"({"field-id":"1","labels":{}})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}, + FieldLabelInvalidParam{ + .test_name = "FieldIdAboveInt32", + .invalid_json_str = R"({"field-id":2147483648,"labels":{}})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}, + FieldLabelInvalidParam{ + .test_name = "FieldIdBelowInt32", + .invalid_json_str = R"({"field-id":-2147483649,"labels":{}})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}, + FieldLabelInvalidParam{.test_name = "MissingLabels", + .invalid_json_str = R"({"field-id":1})", + .expected_error_message = "Missing 'labels'"}, + FieldLabelInvalidParam{.test_name = "NullLabels", + .invalid_json_str = R"({"field-id":1,"labels":null})", + .expected_error_message = "Missing 'labels'"}, + FieldLabelInvalidParam{.test_name = "LabelsNotObject", + .invalid_json_str = R"({"field-id":1,"labels":"x"})", + .expected_error_message = "type must be object"}, + FieldLabelInvalidParam{.test_name = "NonStringLabelValue", + .invalid_json_str = R"({"field-id":1,"labels":{"k":1}})", + .expected_error_message = "type must be string"}), + [](const ::testing::TestParamInfo& info) { + return info.param.test_name; + }); + +DECLARE_ROUNDTRIP_TEST(Labels) + +INSTANTIATE_TEST_SUITE_P( + LabelsCases, LabelsTest, + ::testing::Values( + LabelsParam{.test_name = "Empty", .expected_json_str = R"({})", .model = {}}, + LabelsParam{.test_name = "ObjectLabelsOnly", + .expected_json_str = R"({"object-labels":{"owner":"team-a"}})", + .model = {.object_labels = {{"owner", "team-a"}}}}, + LabelsParam{ + .test_name = "FieldsOnly", + .expected_json_str = + R"({"fields":[{"field-id":1,"labels":{"classification":"pii"}},{"field-id":2,"labels":{"unit":"usd"}}]})", + .model = {.fields = {{.field_id = 1, .labels = {{"classification", "pii"}}}, + {.field_id = 2, .labels = {{"unit", "usd"}}}}}}, + LabelsParam{ + .test_name = "ObjectAndFields", + .expected_json_str = + R"({"object-labels":{"owner":"team-a"},"fields":[{"field-id":1,"labels":{"classification":"pii"}}]})", + .model = {.object_labels = {{"owner", "team-a"}}, + .fields = {{.field_id = 1, + .labels = {{"classification", "pii"}}}}}}, + // Duplicate field IDs are kept verbatim and in order; no resolution + LabelsParam{ + .test_name = "DuplicateFieldIds", + .expected_json_str = + R"({"fields":[{"field-id":1,"labels":{"a":"1"}},{"field-id":1,"labels":{"b":"2"}}]})", + .model = {.fields = {{.field_id = 1, .labels = {{"a", "1"}}}, + {.field_id = 1, .labels = {{"b", "2"}}}}}}), + [](const ::testing::TestParamInfo& info) { + return info.param.test_name; + }); + +DECLARE_DESERIALIZE_TEST(Labels) + +INSTANTIATE_TEST_SUITE_P( + LabelsDeserializeCases, LabelsDeserializeTest, + ::testing::Values( + LabelsDeserializeParam{.test_name = "NullObjectLabels", + .json_str = R"({"object-labels":null})", + .expected_model = {}}, + LabelsDeserializeParam{.test_name = "NullFields", + .json_str = R"({"fields":null})", + .expected_model = {}}, + LabelsDeserializeParam{ + .test_name = "UnknownKeysIgnored", + .json_str = + R"({"object-labels":{"a":"b"},"future":1,"fields":[{"field-id":2,"labels":{},"extra":true}]})", + .expected_model = {.object_labels = {{"a", "b"}}, + .fields = {{.field_id = 2}}}}), + [](const ::testing::TestParamInfo& info) { + return info.param.test_name; + }); + +DECLARE_INVALID_TEST(Labels) + +INSTANTIATE_TEST_SUITE_P( + LabelsInvalidCases, LabelsInvalidTest, + ::testing::Values( + LabelsInvalidParam{.test_name = "ObjectLabelsNotObject", + .invalid_json_str = R"({"object-labels":"x"})", + .expected_error_message = "type must be object"}, + LabelsInvalidParam{.test_name = "ObjectLabelsNonStringValue", + .invalid_json_str = R"({"object-labels":{"k":1}})", + .expected_error_message = "type must be string"}, + LabelsInvalidParam{.test_name = "ObjectLabelsNullValue", + .invalid_json_str = R"({"object-labels":{"k":null}})", + .expected_error_message = "type must be string"}, + LabelsInvalidParam{ + .test_name = "FieldsNotArray", + .invalid_json_str = R"({"fields":{}})", + .expected_error_message = "Cannot parse 'fields' from non-array"}, + LabelsInvalidParam{ + .test_name = "FieldEntryNotObject", + .invalid_json_str = R"({"fields":[1]})", + .expected_error_message = "Cannot parse field label from non-object"}, + LabelsInvalidParam{ + .test_name = "FieldEntryNull", + .invalid_json_str = R"({"fields":[null]})", + .expected_error_message = "Cannot parse field label from non-object"}, + LabelsInvalidParam{ + .test_name = "FieldEntryInvalidFieldId", + .invalid_json_str = R"({"fields":[{"field-id":1.5,"labels":{}}]})", + .expected_error_message = "Cannot parse 'field-id' to an int32 value"}), + [](const ::testing::TestParamInfo& info) { + return info.param.test_name; + }); + } // namespace iceberg::rest diff --git a/src/iceberg/test/table_test.cc b/src/iceberg/test/table_test.cc index a85dc3014..6121fc55b 100644 --- a/src/iceberg/test/table_test.cc +++ b/src/iceberg/test/table_test.cc @@ -22,6 +22,7 @@ #include #include +#include "iceberg/labels.h" #include "iceberg/schema.h" #include "iceberg/schema_field.h" #include "iceberg/table_metadata.h" @@ -148,6 +149,55 @@ TYPED_TEST(TypedTableTest, NewTransaction) { } } +TYPED_TEST(TypedTableTest, LabelsDefaultToEmpty) { + ICEBERG_UNWRAP_OR_FAIL(auto table, this->MakeTable("test_table")); + + EXPECT_TRUE(table->labels().empty()); +} + +class TableLabelsTest : public ::testing::Test { + protected: + void SetUp() override { + io_ = std::make_shared(); + catalog_ = std::make_shared(); + auto schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int64())}, 1); + metadata_ = std::make_shared( + TableMetadata{.format_version = 2, .schemas = {schema}, .current_schema_id = 1}); + } + + const TableIdentifier ident_{.ns = Namespace{.levels = {"db"}}, .name = "t"}; + const Labels labels_{ + .object_labels = {{"owner", "team-a"}}, + .fields = {{.field_id = 1, .labels = {{"classification", "pii"}}}}}; + std::shared_ptr io_; + std::shared_ptr catalog_; + std::shared_ptr metadata_; +}; + +TEST_F(TableLabelsTest, ExposesProvidedLabels) { + ICEBERG_UNWRAP_OR_FAIL( + auto table, Table::Make(ident_, metadata_, "s3://bucket/meta.json", io_, catalog_, + /*full_name=*/"", /*reporter=*/nullptr, labels_)); + + EXPECT_EQ(table->labels(), labels_); +} + +TEST_F(TableLabelsTest, RefreshKeepsLabels) { + ICEBERG_UNWRAP_OR_FAIL( + auto table, Table::Make(ident_, metadata_, "s3://bucket/meta.json", io_, catalog_, + /*full_name=*/"", /*reporter=*/nullptr, labels_)); + ICEBERG_UNWRAP_OR_FAIL( + auto refreshed, + Table::Make(ident_, metadata_, "s3://bucket/meta2.json", io_, catalog_)); + EXPECT_CALL(*catalog_, LoadTable(::testing::_)).WillOnce(::testing::Return(refreshed)); + + ASSERT_THAT(table->Refresh(), IsOk()); + + EXPECT_EQ(table->metadata_file_location(), "s3://bucket/meta2.json"); + EXPECT_EQ(table->labels(), labels_); +} + TEST(StaticTableTest, NewMutatingOperationsAreNotSupported) { auto io = std::make_shared(); auto schema = std::make_shared( diff --git a/src/iceberg/type_fwd.h b/src/iceberg/type_fwd.h index f45dd0115..1f1827773 100644 --- a/src/iceberg/type_fwd.h +++ b/src/iceberg/type_fwd.h @@ -240,6 +240,8 @@ class CommitMetrics; /// \brief Table. class Table; class TableProperties; +struct FieldLabel; +struct Labels; /// \brief Table update. class TableMetadataBuilder; diff --git a/src/iceberg/util/json_util_internal.h b/src/iceberg/util/json_util_internal.h index 65764c4cd..54e1c17ed 100644 --- a/src/iceberg/util/json_util_internal.h +++ b/src/iceberg/util/json_util_internal.h @@ -19,6 +19,8 @@ #pragma once +#include +#include #include #include @@ -105,6 +107,27 @@ Result GetJsonValueOrDefault(const nlohmann::json& json, std::string_view key return GetJsonValueImpl(json, key); } +/// \brief Parse a required int32 value, rejecting floats, booleans and values outside +/// the int32 range, all of which GetJsonValue would silently coerce. +inline Result GetJsonInt32Strict(const nlohmann::json& json, + std::string_view key) { + if (!json.contains(key) || json.at(key).is_null()) { + return JsonParseError("Missing '{}' in {}", key, SafeDumpJson(json)); + } + const auto& value = json.at(key); + constexpr auto kMin = std::numeric_limits::min(); + constexpr auto kMax = std::numeric_limits::max(); + bool in_range = value.is_number_unsigned() + ? value.get() <= static_cast(kMax) + : value.is_number_integer() && value.get() >= kMin && + value.get() <= kMax; + if (!in_range) { + return JsonParseError("Cannot parse '{}' to an int32 value: {}", key, + SafeDumpJson(value)); + } + return value.get(); +} + /// \brief Convert a list of items to a json array. /// /// Note that ToJson(const T&) is required for this function to work.