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..406a3ca6c 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,12 @@ 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 +870,12 @@ 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 +891,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 {}; } 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",