Skip to content

feat(rest): fall back to storage metadata when inline metadata is unusable #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mmaslankaprv merged 2 commits into from
Jul 7, 2026
Merged

feat(rest): fall back to storage metadata when inline metadata is unusable #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e67461b
feat(rest): fall back to storage metadata when inline metadata is unu…
mmaslankaprv Jul 7, 2026
1c66ebf
test(rest): update metadata-fallback tests and fix formatting
mmaslankaprv Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/iceberg/catalog/rest/json_serde.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,16 @@ Result<LoadTableResult> LoadTableResultFromJson(const nlohmann::json& json) {
GetJsonValueOrDefault<std::string>(json, kMetadataLocation));
ICEBERG_ASSIGN_OR_RAISE(auto metadata_json,
GetJsonValue<nlohmann::json>(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<Error>(metadata.error());
}
ICEBERG_ASSIGN_OR_RAISE(result.config,
GetJsonValueOrDefault<decltype(result.config)>(json, kConfig));
if (auto it = json.find(kStorageCredentials); it != json.end() && !it->is_null()) {
Expand Down Expand Up @@ -984,7 +993,11 @@ Result<CommitTableResponse> CommitTableResponseFromJson(const nlohmann::json& js
GetJsonValue<std::string>(json, kMetadataLocation));
ICEBERG_ASSIGN_OR_RAISE(auto metadata_json,
GetJsonValue<nlohmann::json>(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;
}
Expand Down
20 changes: 20 additions & 0 deletions src/iceberg/catalog/rest/rest_catalog.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -747,6 +748,12 @@ Result<std::shared_ptr<Transaction>> 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)));
Expand Down Expand Up @@ -863,6 +870,12 @@ Result<std::shared_ptr<Table>> 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)));
Expand All @@ -878,6 +891,13 @@ Result<std::shared_ptr<Table>> RestCatalog::MakeTableFromCommitResponse(
const SessionContext& context,
const std::unordered_map<std::string, std::string>& table_config,
std::shared_ptr<auth::AuthSession> table_session, std::shared_ptr<FileIO> 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<TableScopedCatalog>(
shared_from_this(), context, identifier, table_config, table_session, table_io);
Expand Down
18 changes: 10 additions & 8 deletions src/iceberg/catalog/rest/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<TableMetadata> 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<TableMetadata> metadata;
std::unordered_map<std::string, std::string> config;
/// \brief Vended storage credentials, one per URI prefix; empty if none.
std::vector<StorageCredential> 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());
Expand Down Expand Up @@ -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<TableMetadata> metadata; // required
std::string metadata_location; // required
/// \brief Required by the spec, but may be null when the inline metadata

@joe-redpanda joe-redpanda Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Glue is out of spec?

@mmaslankaprv mmaslankaprv Jul 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, :-(

/// was unusable; the catalog then loads the metadata file from storage.
std::shared_ptr<TableMetadata> 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 {};
}

Expand Down
30 changes: 22 additions & 8 deletions src/iceberg/test/rest_json_serde_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<LoadTableResultDeserializeParam>& info) {
return info.param.test_name;
});
Expand Down Expand Up @@ -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<CommitTableResponseDeserializeParam>& info) {
return info.param.test_name;
});
Expand Down Expand Up @@ -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",
Expand Down
Loading