From e67461b56aec0c68babbc7194e08b7eb59c765a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ma=C5=9Blanka?= Date: Tue, 7 Jul 2026 13:13:35 +0200 Subject: [PATCH 1/2] feat(rest): fall back to storage metadata when inline metadata is unusable AWS Glue's REST catalog returns inline table metadata that omits required fields such as "schemas", which made LoadTableResult and CommitTableResponse parsing fail even though a valid metadata-location was present. Tolerate unusable inline metadata during JSON parsing: leave metadata null when a metadata-location is available, and have the REST catalog load the complete metadata file from storage via TableMetadataUtil::Read. Validation now only rejects null metadata when no metadata-location is present. --- src/iceberg/catalog/rest/json_serde.cc | 17 +++++++++++++++-- src/iceberg/catalog/rest/rest_catalog.cc | 22 ++++++++++++++++++++++ src/iceberg/catalog/rest/types.h | 18 ++++++++++-------- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/iceberg/catalog/rest/json_serde.cc b/src/iceberg/catalog/rest/json_serde.cc index 4dcfad399..90ace3ada 100644 --- a/src/iceberg/catalog/rest/json_serde.cc +++ b/src/iceberg/catalog/rest/json_serde.cc @@ -731,7 +731,16 @@ Result LoadTableResultFromJson(const nlohmann::json& json) { GetJsonValueOrDefault(json, kMetadataLocation)); ICEBERG_ASSIGN_OR_RAISE(auto metadata_json, GetJsonValue(json, kMetadata)); - ICEBERG_ASSIGN_OR_RAISE(result.metadata, TableMetadataFromJson(metadata_json)); + // Some catalogs (AWS Glue) return inline metadata that omits required + // fields such as "schemas". When the response carries a metadata-location, + // leave metadata null and let the caller load the complete metadata file + // from storage instead of failing the whole request. + auto metadata = TableMetadataFromJson(metadata_json); + if (metadata.has_value()) { + result.metadata = std::move(metadata.value()); + } else if (result.metadata_location.empty()) { + return std::unexpected(metadata.error()); + } ICEBERG_ASSIGN_OR_RAISE(result.config, GetJsonValueOrDefault(json, kConfig)); if (auto it = json.find(kStorageCredentials); it != json.end() && !it->is_null()) { @@ -984,7 +993,11 @@ Result CommitTableResponseFromJson(const nlohmann::json& js GetJsonValue(json, kMetadataLocation)); ICEBERG_ASSIGN_OR_RAISE(auto metadata_json, GetJsonValue(json, kMetadata)); - ICEBERG_ASSIGN_OR_RAISE(response.metadata, TableMetadataFromJson(metadata_json)); + // Tolerate unusable inline metadata (AWS Glue omits "schemas"); the + // metadata-location above is required, so the caller can reload from it. + if (auto metadata = TableMetadataFromJson(metadata_json); metadata.has_value()) { + response.metadata = std::move(metadata.value()); + } ICEBERG_RETURN_UNEXPECTED(response.Validate()); return response; } diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index e7e642f72..9c3a5614a 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -46,6 +46,7 @@ #include "iceberg/schema.h" #include "iceberg/sort_order.h" #include "iceberg/table.h" +#include "iceberg/table_metadata.h" #include "iceberg/table_requirement.h" #include "iceberg/table_requirements.h" #include "iceberg/table_update.h" @@ -747,6 +748,13 @@ Result> RestCatalog::StageCreateTable( auto storage_credentials = std::move(result.storage_credentials); ICEBERG_ASSIGN_OR_RAISE(auto table_io, TableFileIO(context, table_config, storage_credentials)); + if (!result.metadata) { + // The inline metadata was unusable (e.g. AWS Glue omits "schemas"); + // load the complete metadata file pointed to by metadata-location. + ICEBERG_ASSIGN_OR_RAISE( + result.metadata, + TableMetadataUtil::Read(*table_io, result.metadata_location)); + } ICEBERG_ASSIGN_OR_RAISE( auto table_session, TableAuthSession(identifier, table_config, std::move(contextual_session))); @@ -863,6 +871,13 @@ Result> RestCatalog::MakeTableFromLoadResult( auto storage_credentials = std::move(result.storage_credentials); ICEBERG_ASSIGN_OR_RAISE(auto table_io, TableFileIO(context, table_config, storage_credentials)); + if (!result.metadata) { + // The inline metadata was unusable (e.g. AWS Glue omits "schemas"); + // load the complete metadata file pointed to by metadata-location. + ICEBERG_ASSIGN_OR_RAISE( + result.metadata, + TableMetadataUtil::Read(*table_io, result.metadata_location)); + } ICEBERG_ASSIGN_OR_RAISE( auto table_session, TableAuthSession(identifier, table_config, std::move(contextual_session))); @@ -878,6 +893,13 @@ Result> RestCatalog::MakeTableFromCommitResponse( const SessionContext& context, const std::unordered_map& table_config, std::shared_ptr table_session, std::shared_ptr table_io) { + if (!response.metadata) { + // The inline metadata was unusable (e.g. AWS Glue omits "schemas"); + // load the complete metadata file pointed to by metadata-location. + ICEBERG_ASSIGN_OR_RAISE( + response.metadata, + TableMetadataUtil::Read(*table_io, response.metadata_location)); + } // Reuse the bound FileIO because commit responses carry no config or credentials. auto table_catalog = std::make_shared( shared_from_this(), context, identifier, table_config, table_session, table_io); diff --git a/src/iceberg/catalog/rest/types.h b/src/iceberg/catalog/rest/types.h index 20a59fa59..a0b8f0ade 100644 --- a/src/iceberg/catalog/rest/types.h +++ b/src/iceberg/catalog/rest/types.h @@ -184,15 +184,18 @@ using PageToken = std::string; /// \brief Result body for table create/load/register APIs. struct ICEBERG_REST_EXPORT LoadTableResult { std::string metadata_location; - std::shared_ptr metadata; // required + /// \brief Required by the spec, but may be null when the inline metadata + /// was unusable and metadata_location is set; the catalog then loads the + /// metadata file from storage. + std::shared_ptr metadata; std::unordered_map config; /// \brief Vended storage credentials, one per URI prefix; empty if none. std::vector storage_credentials; /// \brief Validates the LoadTableResult. Status Validate() const { - if (!metadata) { - return ValidationFailed("Invalid metadata: null"); + if (!metadata && metadata_location.empty()) { + return ValidationFailed("Invalid metadata: null and no metadata-location"); } for (const auto& credential : storage_credentials) { ICEBERG_RETURN_UNEXPECTED(credential.Validate()); @@ -279,17 +282,16 @@ struct ICEBERG_REST_EXPORT CommitTableRequest { /// \brief Response from committing changes to a table. struct ICEBERG_REST_EXPORT CommitTableResponse { - std::string metadata_location; // required - std::shared_ptr metadata; // required + std::string metadata_location; // required + /// \brief Required by the spec, but may be null when the inline metadata + /// was unusable; the catalog then loads the metadata file from storage. + std::shared_ptr metadata; /// \brief Validates the CommitTableResponse. Status Validate() const { if (metadata_location.empty()) { return ValidationFailed("Invalid metadata location: empty"); } - if (!metadata) { - return ValidationFailed("Invalid metadata: null"); - } return {}; } From 1c66ebf2b28524db7edcc6d4d99630c056a0e96a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ma=C5=9Blanka?= Date: Tue, 7 Jul 2026 13:47:54 +0200 Subject: [PATCH 2/2] test(rest): update metadata-fallback tests and fix formatting - Apply clang-format to the ICEBERG_ASSIGN_OR_RAISE fallback calls in rest_catalog.cc to satisfy the pre-commit clang-format hook. - CommitTableResponse now tolerates unusable inline metadata when a metadata-location is present, so the "WrongMetadataType" invalid case no longer errors. Move that scenario out of the invalid suite and add positive deserialize cases for both LoadTableResult and CommitTableResponse documenting that bad inline metadata + a metadata-location parses to null metadata (reloaded from storage). --- src/iceberg/catalog/rest/rest_catalog.cc | 10 ++++---- src/iceberg/test/rest_json_serde_test.cc | 30 +++++++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index 9c3a5614a..406a3ca6c 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -751,9 +751,8 @@ Result> RestCatalog::StageCreateTable( if (!result.metadata) { // The inline metadata was unusable (e.g. AWS Glue omits "schemas"); // load the complete metadata file pointed to by metadata-location. - ICEBERG_ASSIGN_OR_RAISE( - result.metadata, - TableMetadataUtil::Read(*table_io, result.metadata_location)); + ICEBERG_ASSIGN_OR_RAISE(result.metadata, + TableMetadataUtil::Read(*table_io, result.metadata_location)); } ICEBERG_ASSIGN_OR_RAISE( auto table_session, @@ -874,9 +873,8 @@ Result> RestCatalog::MakeTableFromLoadResult( if (!result.metadata) { // The inline metadata was unusable (e.g. AWS Glue omits "schemas"); // load the complete metadata file pointed to by metadata-location. - ICEBERG_ASSIGN_OR_RAISE( - result.metadata, - TableMetadataUtil::Read(*table_io, result.metadata_location)); + ICEBERG_ASSIGN_OR_RAISE(result.metadata, + TableMetadataUtil::Read(*table_io, result.metadata_location)); } ICEBERG_ASSIGN_OR_RAISE( auto table_session, diff --git a/src/iceberg/test/rest_json_serde_test.cc b/src/iceberg/test/rest_json_serde_test.cc index dd9326cb2..eceb17158 100644 --- a/src/iceberg/test/rest_json_serde_test.cc +++ b/src/iceberg/test/rest_json_serde_test.cc @@ -1162,7 +1162,17 @@ 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"}}}}, + // Unusable inline metadata (e.g. AWS Glue returns metadata that is not + // an object, or omits required fields). Because a metadata-location is + // present, parsing succeeds with null metadata so the catalog can load + // the complete metadata file from storage. + LoadTableResultDeserializeParam{ + .test_name = "UnusableInlineMetadata", + .json_str = + R"({"metadata-location":"s3://bucket/metadata/v1.json","metadata":"invalid"})", + .expected_model = {.metadata_location = "s3://bucket/metadata/v1.json", + .metadata = nullptr}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -1400,7 +1410,17 @@ INSTANTIATE_TEST_SUITE_P( .json_str = R"({"metadata-location":"s3://bucket/metadata/v2.json","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":{}}})", .expected_model = {.metadata_location = "s3://bucket/metadata/v2.json", - .metadata = MakeSimpleTableMetadata()}}), + .metadata = MakeSimpleTableMetadata()}}, + // Unusable inline metadata (e.g. AWS Glue returns metadata that is not + // an object, or omits required fields). Because a metadata-location is + // present, parsing succeeds with null metadata so the catalog can load + // the complete metadata file from storage. + CommitTableResponseDeserializeParam{ + .test_name = "UnusableInlineMetadata", + .json_str = + R"({"metadata-location":"s3://bucket/metadata/v2.json","metadata":"invalid"})", + .expected_model = {.metadata_location = "s3://bucket/metadata/v2.json", + .metadata = nullptr}}), [](const ::testing::TestParamInfo& info) { return info.param.test_name; }); @@ -1433,12 +1453,6 @@ INSTANTIATE_TEST_SUITE_P( .invalid_json_str = R"({"metadata-location":123,"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}})", .expected_error_message = "type must be string, but is number"}, - // Wrong type for metadata field - CommitTableResponseInvalidParam{ - .test_name = "WrongMetadataType", - .invalid_json_str = - R"({"metadata-location":"s3://bucket/metadata/v2.json","metadata":"invalid"})", - .expected_error_message = "Cannot parse metadata from a non-object"}, // Empty JSON object CommitTableResponseInvalidParam{ .test_name = "EmptyJson",