Skip to content

Commit 90a2be7

Browse files
authored
refactor(serde): return Result from ToJson schema/type/metadata serializers (#785)
## What Change the schema/type/metadata `ToJson` serializers from returning bare `nlohmann::json` to `Result<nlohmann::json>`: - `json_serde`: `ToJson(SchemaField | Type | Schema | TableMetadata | TableUpdate)` - REST: `ToJson(CreateTableRequest | CommitTableRequest | LoadTableResult | CommitTableResponse)` Errors propagate via `ICEBERG_ASSIGN_OR_RAISE`; callers bottom out at the existing `Result`-returning boundaries (`ToJsonString`, `rest_catalog`, `TableMetadataUtil::Write`). ## Why Preparation for v3 column default values (#730 / #746). Single-value (`Literal`) serialization is fallible, and column defaults invoke it from schema serialization, so these serializers need to propagate the error instead of throwing. Splitting it out keeps the feature PR focused on the feature. ## Notes - **No behavior change** — every conversion still succeeds today; only the return type changes. - The shared `ToJsonList` template stays bare (it also serializes infallible types such as partition specs and snapshots); `TableMetadata` serializes its schema list with an explicit loop. - The REST `ICEBERG_DECLARE_JSON_SERDE` macro is unchanged; the four schema/metadata-bearing models are declared explicitly so only their `ToJson` return type differs. ## Testing No behavior change; existing tests are adapted to the new return type. Full build and `ctest` pass locally.
1 parent 9a7439c commit 90a2be7

9 files changed

Lines changed: 120 additions & 67 deletions

File tree

src/iceberg/catalog/rest/json_serde.cc

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -690,10 +690,10 @@ Result<RenameTableRequest> RenameTableRequestFromJson(const nlohmann::json& json
690690
}
691691

692692
// LoadTableResult (used by CreateTableResponse, LoadTableResponse)
693-
nlohmann::json ToJson(const LoadTableResult& result) {
693+
Result<nlohmann::json> ToJson(const LoadTableResult& result) {
694694
nlohmann::json json;
695695
SetOptionalStringField(json, kMetadataLocation, result.metadata_location);
696-
json[kMetadata] = ToJson(*result.metadata);
696+
ICEBERG_ASSIGN_OR_RAISE(json[kMetadata], ToJson(*result.metadata));
697697
SetContainerField(json, kConfig, result.config);
698698
return json;
699699
}
@@ -820,12 +820,12 @@ Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json
820820
return response;
821821
}
822822

823-
nlohmann::json ToJson(const CreateTableRequest& request) {
823+
Result<nlohmann::json> ToJson(const CreateTableRequest& request) {
824824
nlohmann::json json;
825825
json[kName] = request.name;
826826
SetOptionalStringField(json, kLocation, request.location);
827827
if (request.schema) {
828-
json[kSchema] = ToJson(*request.schema);
828+
ICEBERG_ASSIGN_OR_RAISE(json[kSchema], ToJson(*request.schema));
829829
}
830830
if (request.partition_spec) {
831831
json[kPartitionSpec] = ToJson(*request.partition_spec);
@@ -872,7 +872,7 @@ Result<CreateTableRequest> CreateTableRequestFromJson(const nlohmann::json& json
872872
}
873873

874874
// CommitTableRequest serialization
875-
nlohmann::json ToJson(const CommitTableRequest& request) {
875+
Result<nlohmann::json> ToJson(const CommitTableRequest& request) {
876876
nlohmann::json json;
877877
if (!request.identifier.name.empty()) {
878878
json[kIdentifier] = ToJson(request.identifier);
@@ -886,7 +886,8 @@ nlohmann::json ToJson(const CommitTableRequest& request) {
886886

887887
nlohmann::json updates_json = nlohmann::json::array();
888888
for (const auto& update : request.updates) {
889-
updates_json.push_back(ToJson(*update));
889+
ICEBERG_ASSIGN_OR_RAISE(auto update_json, ToJson(*update));
890+
updates_json.push_back(std::move(update_json));
890891
}
891892
json[kUpdates] = std::move(updates_json);
892893

@@ -932,11 +933,11 @@ Result<CommitTableRequest> CommitTableRequestFromJson(const nlohmann::json& json
932933
}
933934

934935
// CommitTableResponse serialization
935-
nlohmann::json ToJson(const CommitTableResponse& response) {
936+
Result<nlohmann::json> ToJson(const CommitTableResponse& response) {
936937
nlohmann::json json;
937938
json[kMetadataLocation] = response.metadata_location;
938939
if (response.metadata) {
939-
json[kMetadata] = ToJson(*response.metadata);
940+
ICEBERG_ASSIGN_OR_RAISE(json[kMetadata], ToJson(*response.metadata));
940941
}
941942
return json;
942943
}

src/iceberg/catalog/rest/json_serde_internal.h

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,38 @@ ICEBERG_DECLARE_JSON_SERDE(GetNamespaceResponse)
5757
ICEBERG_DECLARE_JSON_SERDE(UpdateNamespacePropertiesRequest)
5858
ICEBERG_DECLARE_JSON_SERDE(UpdateNamespacePropertiesResponse)
5959
ICEBERG_DECLARE_JSON_SERDE(ListTablesResponse)
60-
ICEBERG_DECLARE_JSON_SERDE(LoadTableResult)
6160
ICEBERG_DECLARE_JSON_SERDE(RegisterTableRequest)
6261
ICEBERG_DECLARE_JSON_SERDE(RenameTableRequest)
63-
ICEBERG_DECLARE_JSON_SERDE(CreateTableRequest)
64-
ICEBERG_DECLARE_JSON_SERDE(CommitTableRequest)
65-
ICEBERG_DECLARE_JSON_SERDE(CommitTableResponse)
6662
ICEBERG_DECLARE_JSON_SERDE(OAuthTokenResponse)
6763

6864
#undef ICEBERG_DECLARE_JSON_SERDE
6965

66+
// These models embed a Schema/TableMetadata whose ToJson returns Result, so their own
67+
// ToJson returns Result too. FromJson is declared like the macro-based models above.
68+
ICEBERG_REST_EXPORT Result<LoadTableResult> LoadTableResultFromJson(
69+
const nlohmann::json& json);
70+
template <>
71+
ICEBERG_REST_EXPORT Result<LoadTableResult> FromJson(const nlohmann::json& json);
72+
ICEBERG_REST_EXPORT Result<nlohmann::json> ToJson(const LoadTableResult& model);
73+
74+
ICEBERG_REST_EXPORT Result<CreateTableRequest> CreateTableRequestFromJson(
75+
const nlohmann::json& json);
76+
template <>
77+
ICEBERG_REST_EXPORT Result<CreateTableRequest> FromJson(const nlohmann::json& json);
78+
ICEBERG_REST_EXPORT Result<nlohmann::json> ToJson(const CreateTableRequest& model);
79+
80+
ICEBERG_REST_EXPORT Result<CommitTableRequest> CommitTableRequestFromJson(
81+
const nlohmann::json& json);
82+
template <>
83+
ICEBERG_REST_EXPORT Result<CommitTableRequest> FromJson(const nlohmann::json& json);
84+
ICEBERG_REST_EXPORT Result<nlohmann::json> ToJson(const CommitTableRequest& model);
85+
86+
ICEBERG_REST_EXPORT Result<CommitTableResponse> CommitTableResponseFromJson(
87+
const nlohmann::json& json);
88+
template <>
89+
ICEBERG_REST_EXPORT Result<CommitTableResponse> FromJson(const nlohmann::json& json);
90+
ICEBERG_REST_EXPORT Result<nlohmann::json> ToJson(const CommitTableResponse& model);
91+
7092
ICEBERG_REST_EXPORT Result<PlanTableScanResponse> PlanTableScanResponseFromJson(
7193
const nlohmann::json& json,
7294
const std::unordered_map<int32_t, std::shared_ptr<PartitionSpec>>&

src/iceberg/catalog/rest/rest_catalog.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,8 @@ Result<LoadTableResult> RestCatalog::CreateTableInternal(
667667
.properties = properties,
668668
};
669669

670-
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request)));
670+
ICEBERG_ASSIGN_OR_RAISE(auto request_json, ToJson(request));
671+
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(request_json));
671672
ICEBERG_ASSIGN_OR_RAISE(const auto response,
672673
client_->Post(path, json_request, /*headers=*/{},
673674
*TableErrorHandler::Instance(), session));
@@ -710,7 +711,8 @@ Result<CommitTableResponse> RestCatalog::UpdateTableInternal(
710711
request.updates.push_back(update->Clone());
711712
}
712713

713-
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request)));
714+
ICEBERG_ASSIGN_OR_RAISE(auto request_json, ToJson(request));
715+
ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(request_json));
714716
ICEBERG_ASSIGN_OR_RAISE(auto is_create, TableRequirements::IsCreate(requirements));
715717
const ErrorHandler* error_handler = TableCommitErrorHandler::Instance().get();
716718
if (is_create) {

src/iceberg/json_serde.cc

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -315,27 +315,28 @@ Result<std::unique_ptr<SortOrder>> SortOrderFromJson(const nlohmann::json& json)
315315
return SortOrder::Make(parsed.order_id, std::move(parsed.fields));
316316
}
317317

318-
nlohmann::json ToJson(const SchemaField& field) {
318+
Result<nlohmann::json> ToJson(const SchemaField& field) {
319319
nlohmann::json json;
320320
json[kId] = field.field_id();
321321
json[kName] = field.name();
322322
json[kRequired] = !field.optional();
323-
json[kType] = ToJson(*field.type());
323+
ICEBERG_ASSIGN_OR_RAISE(json[kType], ToJson(*field.type()));
324324
if (!field.doc().empty()) {
325325
json[kDoc] = field.doc();
326326
}
327327
return json;
328328
}
329329

330-
nlohmann::json ToJson(const Type& type) {
330+
Result<nlohmann::json> ToJson(const Type& type) {
331331
switch (type.type_id()) {
332332
case TypeId::kStruct: {
333333
const auto& struct_type = internal::checked_cast<const StructType&>(type);
334334
nlohmann::json json;
335335
json[kType] = kStruct;
336336
nlohmann::json fields_json = nlohmann::json::array();
337337
for (const auto& field : struct_type.fields()) {
338-
fields_json.push_back(ToJson(field));
338+
ICEBERG_ASSIGN_OR_RAISE(auto field_json, ToJson(field));
339+
fields_json.push_back(std::move(field_json));
339340
// TODO(gangwu): add default values
340341
}
341342
json[kFields] = fields_json;
@@ -349,7 +350,7 @@ nlohmann::json ToJson(const Type& type) {
349350
const auto& element_field = list_type.fields().front();
350351
json[kElementId] = element_field.field_id();
351352
json[kElementRequired] = !element_field.optional();
352-
json[kElement] = ToJson(*element_field.type());
353+
ICEBERG_ASSIGN_OR_RAISE(json[kElement], ToJson(*element_field.type()));
353354
return json;
354355
}
355356
case TypeId::kMap: {
@@ -359,12 +360,12 @@ nlohmann::json ToJson(const Type& type) {
359360

360361
const auto& key_field = map_type.key();
361362
json[kKeyId] = key_field.field_id();
362-
json[kKey] = ToJson(*key_field.type());
363+
ICEBERG_ASSIGN_OR_RAISE(json[kKey], ToJson(*key_field.type()));
363364

364365
const auto& value_field = map_type.value();
365366
json[kValueId] = value_field.field_id();
366367
json[kValueRequired] = !value_field.optional();
367-
json[kValue] = ToJson(*value_field.type());
368+
ICEBERG_ASSIGN_OR_RAISE(json[kValue], ToJson(*value_field.type()));
368369
return json;
369370
}
370371
case TypeId::kBoolean:
@@ -416,8 +417,9 @@ nlohmann::json ToJson(const Type& type) {
416417
std::unreachable();
417418
}
418419

419-
nlohmann::json ToJson(const Schema& schema) {
420-
nlohmann::json json = ToJson(internal::checked_cast<const Type&>(schema));
420+
Result<nlohmann::json> ToJson(const Schema& schema) {
421+
ICEBERG_ASSIGN_OR_RAISE(nlohmann::json json,
422+
ToJson(internal::checked_cast<const Type&>(schema)));
421423
json[kSchemaId] = schema.schema_id();
422424
if (!schema.IdentifierFieldIds().empty()) {
423425
json[kIdentifierFieldIds] = schema.IdentifierFieldIds();
@@ -426,7 +428,8 @@ nlohmann::json ToJson(const Schema& schema) {
426428
}
427429

428430
Result<std::string> ToJsonString(const Schema& schema) {
429-
return ToJsonString(ToJson(schema));
431+
ICEBERG_ASSIGN_OR_RAISE(auto json, ToJson(schema));
432+
return ToJsonString(json);
430433
}
431434

432435
nlohmann::json ToJson(const SnapshotRef& ref) {
@@ -966,7 +969,7 @@ Result<EncryptedKey> EncryptedKeyFromJson(const nlohmann::json& json) {
966969
};
967970
}
968971

969-
nlohmann::json ToJson(const TableMetadata& table_metadata) {
972+
Result<nlohmann::json> ToJson(const TableMetadata& table_metadata) {
970973
nlohmann::json json;
971974

972975
json[kFormatVersion] = table_metadata.format_version;
@@ -984,15 +987,22 @@ nlohmann::json ToJson(const TableMetadata& table_metadata) {
984987
if (table_metadata.format_version == 1) {
985988
for (const auto& schema : table_metadata.schemas) {
986989
if (schema->schema_id() == table_metadata.current_schema_id) {
987-
json[kSchema] = ToJson(*schema);
990+
ICEBERG_ASSIGN_OR_RAISE(json[kSchema], ToJson(*schema));
988991
break;
989992
}
990993
}
991994
}
992995

993996
// write the current schema ID and schema list
994997
json[kCurrentSchemaId] = table_metadata.current_schema_id;
995-
json[kSchemas] = ToJsonList(table_metadata.schemas);
998+
// ToJson(Schema) is fallible, so the shared ToJsonList helper (which assumes an
999+
// infallible ToJson) cannot be used here; build the array with an explicit loop.
1000+
nlohmann::json schemas_json = nlohmann::json::array();
1001+
for (const auto& schema : table_metadata.schemas) {
1002+
ICEBERG_ASSIGN_OR_RAISE(auto schema_json, ToJson(*schema));
1003+
schemas_json.push_back(std::move(schema_json));
1004+
}
1005+
json[kSchemas] = std::move(schemas_json);
9961006

9971007
// for older readers, continue writing the default spec as "partition-spec"
9981008
if (table_metadata.format_version == 1) {
@@ -1042,7 +1052,8 @@ nlohmann::json ToJson(const TableMetadata& table_metadata) {
10421052
}
10431053

10441054
Result<std::string> ToJsonString(const TableMetadata& table_metadata) {
1045-
return ToJsonString(ToJson(table_metadata));
1055+
ICEBERG_ASSIGN_OR_RAISE(auto json, ToJson(table_metadata));
1056+
return ToJsonString(json);
10461057
}
10471058

10481059
namespace {
@@ -1446,7 +1457,7 @@ Result<Namespace> NamespaceFromJson(const nlohmann::json& json) {
14461457
return ns;
14471458
}
14481459

1449-
nlohmann::json ToJson(const TableUpdate& update) {
1460+
Result<nlohmann::json> ToJson(const TableUpdate& update) {
14501461
nlohmann::json json;
14511462
switch (update.kind()) {
14521463
case TableUpdate::Kind::kAssignUUID: {
@@ -1465,7 +1476,7 @@ nlohmann::json ToJson(const TableUpdate& update) {
14651476
const auto& u = internal::checked_cast<const table::AddSchema&>(update);
14661477
json[kAction] = kActionAddSchema;
14671478
if (u.schema()) {
1468-
json[kSchema] = ToJson(*u.schema());
1479+
ICEBERG_ASSIGN_OR_RAISE(json[kSchema], ToJson(*u.schema()));
14691480
} else {
14701481
json[kSchema] = nlohmann::json::value_t::null;
14711482
}

src/iceberg/json_serde_internal.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ ICEBERG_EXPORT Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
9393
///
9494
/// \param schema The Iceberg schema to convert.
9595
/// \return The JSON representation of the schema.
96-
ICEBERG_EXPORT nlohmann::json ToJson(const Schema& schema);
96+
ICEBERG_EXPORT Result<nlohmann::json> ToJson(const Schema& schema);
9797

9898
/// \brief Convert an Iceberg Schema to JSON.
9999
///
@@ -111,7 +111,7 @@ ICEBERG_EXPORT Result<std::unique_ptr<Schema>> SchemaFromJson(const nlohmann::js
111111
///
112112
/// \param type The Iceberg type to convert.
113113
/// \return The JSON representation of the type.
114-
ICEBERG_EXPORT nlohmann::json ToJson(const Type& type);
114+
ICEBERG_EXPORT Result<nlohmann::json> ToJson(const Type& type);
115115

116116
/// \brief Convert JSON to an Iceberg Type.
117117
///
@@ -123,7 +123,7 @@ ICEBERG_EXPORT Result<std::unique_ptr<Type>> TypeFromJson(const nlohmann::json&
123123
///
124124
/// \param field The Iceberg field to convert.
125125
/// \return The JSON representation of the field.
126-
ICEBERG_EXPORT nlohmann::json ToJson(const SchemaField& field);
126+
ICEBERG_EXPORT Result<nlohmann::json> ToJson(const SchemaField& field);
127127

128128
/// \brief Convert JSON to an Iceberg SchemaField.
129129
///
@@ -300,7 +300,7 @@ ICEBERG_EXPORT Result<EncryptedKey> EncryptedKeyFromJson(const nlohmann::json& j
300300
///
301301
/// \param table_metadata The `TableMetadata` object to be serialized.
302302
/// \return A JSON object representing the `TableMetadata`.
303-
ICEBERG_EXPORT nlohmann::json ToJson(const TableMetadata& table_metadata);
303+
ICEBERG_EXPORT Result<nlohmann::json> ToJson(const TableMetadata& table_metadata);
304304

305305
/// \brief Serializes a `TableMetadata` object to JSON.
306306
///
@@ -404,7 +404,7 @@ ICEBERG_EXPORT Result<Namespace> NamespaceFromJson(const nlohmann::json& json);
404404
///
405405
/// \param update The `TableUpdate` object to be serialized.
406406
/// \return A JSON object representing the `TableUpdate`.
407-
ICEBERG_EXPORT nlohmann::json ToJson(const TableUpdate& update);
407+
ICEBERG_EXPORT Result<nlohmann::json> ToJson(const TableUpdate& update);
408408

409409
/// \brief Deserializes a JSON object into a `TableUpdate` object.
410410
///

src/iceberg/table_metadata.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ Result<std::string> TableMetadataUtil::Write(FileIO& io, const TableMetadata* ba
489489

490490
Status TableMetadataUtil::Write(FileIO& io, const std::string& location,
491491
const TableMetadata& metadata) {
492-
auto json = ToJson(metadata);
492+
ICEBERG_ASSIGN_OR_RAISE(auto json, ToJson(metadata));
493493
ICEBERG_ASSIGN_OR_RAISE(auto json_string, ToJsonString(json));
494494
return io.WriteFile(location, json_string);
495495
}

0 commit comments

Comments
 (0)