feat: add non-validated version of FromJson for SortOrder and PartitionSpec (#518)

Author: HeartLinked

src/iceberg/json_internal.cc

@@ -33,7 +33,6 @@
 #include "iceberg/partition_spec.h"
 #include "iceberg/result.h"
 #include "iceberg/schema.h"
-#include "iceberg/schema_internal.h"
 #include "iceberg/snapshot.h"
 #include "iceberg/sort_order.h"
 #include "iceberg/statistics_file.h"
@@ -271,6 +270,18 @@ Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
   return SortOrder::Make(*current_schema, order_id, std::move(sort_fields));
 }
 
+Result<std::unique_ptr<SortOrder>> SortOrderFromJson(const nlohmann::json& json) {
+  ICEBERG_ASSIGN_OR_RAISE(auto order_id, GetJsonValue<int32_t>(json, kOrderId));
+  ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue<nlohmann::json>(json, kFields));
+
+  std::vector<SortField> sort_fields;
+  for (const auto& field_json : fields) {
+    ICEBERG_ASSIGN_OR_RAISE(auto sort_field, SortFieldFromJson(field_json));
+    sort_fields.push_back(std::move(*sort_field));
+  }
+  return SortOrder::Make(order_id, std::move(sort_fields));
+}
+
 nlohmann::json ToJson(const SchemaField& field) {
   nlohmann::json json;
   json[kId] = field.field_id();
@@ -615,6 +626,19 @@ Result<std::unique_ptr<PartitionSpec>> PartitionSpecFromJson(
   return spec;
 }
 
+Result<std::unique_ptr<PartitionSpec>> PartitionSpecFromJson(const nlohmann::json& json) {
+  ICEBERG_ASSIGN_OR_RAISE(auto spec_id, GetJsonValue<int32_t>(json, kSpecId));
+  ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue<nlohmann::json>(json, kFields));
+
+  std::vector<PartitionField> partition_fields;
+  for (const auto& field_json : fields) {
+    ICEBERG_ASSIGN_OR_RAISE(auto partition_field, PartitionFieldFromJson(field_json));
+    partition_fields.push_back(std::move(*partition_field));
+  }
+
+  return PartitionSpec::Make(spec_id, std::move(partition_fields));
+}
+
 Result<std::unique_ptr<SnapshotRef>> SnapshotRefFromJson(const nlohmann::json& json) {
   ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue<int64_t>(json, kSnapshotId));
   ICEBERG_ASSIGN_OR_RAISE(
@@ -1492,10 +1516,8 @@ Result<std::unique_ptr<TableUpdate>> TableUpdateFromJson(const nlohmann::json& j
   }
   if (action == kActionAddPartitionSpec) {
     ICEBERG_ASSIGN_OR_RAISE(auto spec_json, GetJsonValue<nlohmann::json>(json, kSpec));
-    ICEBERG_ASSIGN_OR_RAISE(auto spec_id_opt,
-                            GetJsonValueOptional<int32_t>(spec_json, kSpecId));
-    // TODO(Feiyang Li): add fromJson for UnboundPartitionSpec and then use it here
-    return NotImplemented("FromJson of TableUpdate::AddPartitionSpec is not implemented");
+    ICEBERG_ASSIGN_OR_RAISE(auto spec, PartitionSpecFromJson(spec_json));
+    return std::make_unique<table::AddPartitionSpec>(std::move(spec));
   }
   if (action == kActionSetDefaultPartitionSpec) {
     ICEBERG_ASSIGN_OR_RAISE(auto spec_id, GetJsonValue<int32_t>(json, kSpecId));
@@ -1515,8 +1537,8 @@ Result<std::unique_ptr<TableUpdate>> TableUpdateFromJson(const nlohmann::json& j
   if (action == kActionAddSortOrder) {
     ICEBERG_ASSIGN_OR_RAISE(auto sort_order_json,
                             GetJsonValue<nlohmann::json>(json, kSortOrder));
-    // TODO(Feiyang Li): add fromJson for UnboundSortOrder and then use it here
-    return NotImplemented("FromJson of TableUpdate::AddSortOrder is not implemented");
+    ICEBERG_ASSIGN_OR_RAISE(auto sort_order, SortOrderFromJson(sort_order_json));
+    return std::make_unique<table::AddSortOrder>(std::move(sort_order));
   }
   if (action == kActionSetDefaultSortOrder) {
     ICEBERG_ASSIGN_OR_RAISE(auto sort_order_id,

src/iceberg/json_internal.h

@@ -75,6 +75,14 @@ ICEBERG_EXPORT nlohmann::json ToJson(const SortOrder& sort_order);
 ICEBERG_EXPORT Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
     const nlohmann::json& json, const std::shared_ptr<Schema>& current_schema);
 
+/// \brief Deserializes a JSON object into a `SortOrder` object.
+///
+/// \param json The JSON object representing a `SortOrder`.
+/// \return An `expected` value containing either a `SortOrder` object or an error. If the
+/// JSON is malformed or missing expected fields, an error will be returned.
+ICEBERG_EXPORT Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
+    const nlohmann::json& json);
+
 /// \brief Convert an Iceberg Schema to JSON.
 ///
 /// \param schema The Iceberg schema to convert.
@@ -183,6 +191,14 @@ ICEBERG_EXPORT Result<std::unique_ptr<PartitionSpec>> PartitionSpecFromJson(
     const std::shared_ptr<Schema>& schema, const nlohmann::json& json,
     int32_t default_spec_id);
 
+/// \brief Deserializes a JSON object into a `PartitionSpec` object.
+///
+/// \param json The JSON object representing a `PartitionSpec`.
+/// \return An `expected` value containing either a `PartitionSpec` object or an error. If
+/// the JSON is malformed or missing expected fields, an error will be returned.
+ICEBERG_EXPORT Result<std::unique_ptr<PartitionSpec>> PartitionSpecFromJson(
+    const nlohmann::json& json);
+
 /// \brief Serializes a `SnapshotRef` object to JSON.
 ///
 /// \param snapshot_ref The `SnapshotRef` object to be serialized.

src/iceberg/test/json_internal_test.cc

@@ -175,6 +175,29 @@ TEST(JsonInternalTest, PartitionSpec) {
   EXPECT_EQ(*spec, *parsed_spec_result.value());
 }
 
+TEST(JsonInternalTest, SortOrderFromJson) {
+  auto identity_transform = Transform::Identity();
+  SortField st1(5, identity_transform, SortDirection::kAscending, NullOrder::kFirst);
+  SortField st2(7, identity_transform, SortDirection::kDescending, NullOrder::kLast);
+  ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(100, {st1, st2}));
+
+  auto json = ToJson(*sort_order);
+  ICEBERG_UNWRAP_OR_FAIL(auto parsed, SortOrderFromJson(json));
+  EXPECT_EQ(*sort_order, *parsed);
+}
+
+TEST(JsonInternalTest, PartitionSpecFromJson) {
+  auto identity_transform = Transform::Identity();
+  ICEBERG_UNWRAP_OR_FAIL(
+      auto spec,
+      PartitionSpec::Make(1, {PartitionField(3, 101, "region", identity_transform),
+                              PartitionField(5, 102, "ts", identity_transform)}));
+
+  auto json = ToJson(*spec);
+  ICEBERG_UNWRAP_OR_FAIL(auto parsed, PartitionSpecFromJson(json));
+  EXPECT_EQ(*spec, *parsed);
+}
+
 TEST(JsonInternalTest, SnapshotRefBranch) {
   SnapshotRef ref(1234567890, SnapshotRef::Branch{.min_snapshots_to_keep = 10,
                                                   .max_snapshot_age_ms = 123456789,
@@ -349,6 +372,23 @@ TEST(JsonInternalTest, TableUpdateSetCurrentSchema) {
             update);
 }
 
+TEST(JsonInternalTest, TableUpdateAddPartitionSpec) {
+  auto identity_transform = Transform::Identity();
+  ICEBERG_UNWRAP_OR_FAIL(
+      auto spec,
+      PartitionSpec::Make(1, {PartitionField(3, 101, "region", identity_transform)}));
+  table::AddPartitionSpec update(std::move(spec));
+
+  auto json = ToJson(update);
+  EXPECT_EQ(json["action"], "add-spec");
+  EXPECT_TRUE(json.contains("spec"));
+
+  auto parsed = TableUpdateFromJson(json);
+  ASSERT_THAT(parsed, IsOk());
+  auto* actual = internal::checked_cast<table::AddPartitionSpec*>(parsed.value().get());
+  EXPECT_EQ(*actual->spec(), *update.spec());
+}
+
 TEST(JsonInternalTest, TableUpdateSetDefaultPartitionSpec) {
   table::SetDefaultPartitionSpec update(2);
   nlohmann::json expected = R"({"action":"set-default-spec","spec-id":2})"_json;
@@ -386,6 +426,22 @@ TEST(JsonInternalTest, TableUpdateRemoveSchemas) {
   EXPECT_EQ(*internal::checked_cast<table::RemoveSchemas*>(parsed.value().get()), update);
 }
 
+TEST(JsonInternalTest, TableUpdateAddSortOrder) {
+  auto identity_transform = Transform::Identity();
+  SortField st(5, identity_transform, SortDirection::kAscending, NullOrder::kFirst);
+  ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(1, {st}));
+  table::AddSortOrder update(std::move(sort_order));
+
+  auto json = ToJson(update);
+  EXPECT_EQ(json["action"], "add-sort-order");
+  EXPECT_TRUE(json.contains("sort-order"));
+
+  auto parsed = TableUpdateFromJson(json);
+  ASSERT_THAT(parsed, IsOk());
+  auto* actual = internal::checked_cast<table::AddSortOrder*>(parsed.value().get());
+  EXPECT_EQ(*actual->sort_order(), *update.sort_order());
+}
+
 TEST(JsonInternalTest, TableUpdateSetDefaultSortOrder) {
   table::SetDefaultSortOrder update(1);
   nlohmann::json expected =