From 39724bbc729539ed394762ae58724ad6e9b65933 Mon Sep 17 00:00:00 2001 From: Feiyang Li Date: Tue, 20 Jan 2026 17:12:49 +0800 Subject: [PATCH 1/4] feat: add unbound FromJson for SortOrder and PartitionSpec --- src/iceberg/json_internal.cc | 36 +++++++++++++---- src/iceberg/json_internal.h | 22 ++++++++++ src/iceberg/test/json_internal_test.cc | 56 ++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/iceberg/json_internal.cc b/src/iceberg/json_internal.cc index 7e2652d6c..e92d3dd57 100644 --- a/src/iceberg/json_internal.cc +++ b/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> SortOrderFromJson( return SortOrder::Make(*current_schema, order_id, std::move(sort_fields)); } +Result> SortOrderFromJson(const nlohmann::json& json) { + ICEBERG_ASSIGN_OR_RAISE(auto order_id, GetJsonValue(json, kOrderId)); + ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue(json, kFields)); + + std::vector 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> PartitionSpecFromJson( return spec; } +Result> PartitionSpecFromJson(const nlohmann::json& json) { + ICEBERG_ASSIGN_OR_RAISE(auto spec_id, GetJsonValue(json, kSpecId)); + ICEBERG_ASSIGN_OR_RAISE(auto fields, GetJsonValue(json, kFields)); + + std::vector 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> SnapshotRefFromJson(const nlohmann::json& json) { ICEBERG_ASSIGN_OR_RAISE(auto snapshot_id, GetJsonValue(json, kSnapshotId)); ICEBERG_ASSIGN_OR_RAISE( @@ -1492,10 +1516,8 @@ Result> TableUpdateFromJson(const nlohmann::json& j } if (action == kActionAddPartitionSpec) { ICEBERG_ASSIGN_OR_RAISE(auto spec_json, GetJsonValue(json, kSpec)); - ICEBERG_ASSIGN_OR_RAISE(auto spec_id_opt, - GetJsonValueOptional(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(std::move(spec)); } if (action == kActionSetDefaultPartitionSpec) { ICEBERG_ASSIGN_OR_RAISE(auto spec_id, GetJsonValue(json, kSpecId)); @@ -1515,8 +1537,8 @@ Result> TableUpdateFromJson(const nlohmann::json& j if (action == kActionAddSortOrder) { ICEBERG_ASSIGN_OR_RAISE(auto sort_order_json, GetJsonValue(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(std::move(sort_order)); } if (action == kActionSetDefaultSortOrder) { ICEBERG_ASSIGN_OR_RAISE(auto sort_order_id, diff --git a/src/iceberg/json_internal.h b/src/iceberg/json_internal.h index d55252ca0..4d32d6896 100644 --- a/src/iceberg/json_internal.h +++ b/src/iceberg/json_internal.h @@ -75,6 +75,17 @@ ICEBERG_EXPORT nlohmann::json ToJson(const SortOrder& sort_order); ICEBERG_EXPORT Result> SortOrderFromJson( const nlohmann::json& json, const std::shared_ptr& current_schema); +/// \brief Deserializes a JSON object into an unbound `SortOrder` object. +/// +/// This function parses the provided JSON and creates a `SortOrder` object without +/// binding it to a schema. It does not validate the sort fields against any schema. +/// +/// \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> SortOrderFromJson( + const nlohmann::json& json); + /// \brief Convert an Iceberg Schema to JSON. /// /// \param schema The Iceberg schema to convert. @@ -183,6 +194,17 @@ ICEBERG_EXPORT Result> PartitionSpecFromJson( const std::shared_ptr& schema, const nlohmann::json& json, int32_t default_spec_id); +/// \brief Deserializes a JSON object into an unbound `PartitionSpec` object. +/// +/// This function parses the provided JSON and creates a `PartitionSpec` object without +/// binding it to a schema. It does not validate the partition fields against any schema. +/// +/// \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> PartitionSpecFromJson( + const nlohmann::json& json); + /// \brief Serializes a `SnapshotRef` object to JSON. /// /// \param snapshot_ref The `SnapshotRef` object to be serialized. diff --git a/src/iceberg/test/json_internal_test.cc b/src/iceberg/test/json_internal_test.cc index 0b5f4f59b..85a67d06d 100644 --- a/src/iceberg/test/json_internal_test.cc +++ b/src/iceberg/test/json_internal_test.cc @@ -175,6 +175,29 @@ TEST(JsonInternalTest, PartitionSpec) { EXPECT_EQ(*spec, *parsed_spec_result.value()); } +TEST(JsonInternalTest, SortOrderFromJsonUnbound) { + 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, PartitionSpecFromJsonUnbound) { + 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(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(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(parsed.value().get()); + EXPECT_EQ(*actual->sort_order(), *update.sort_order()); +} + TEST(JsonInternalTest, TableUpdateSetDefaultSortOrder) { table::SetDefaultSortOrder update(1); nlohmann::json expected = From 23c1260ac857fd8f682ac4fb228ce3ed653f63aa Mon Sep 17 00:00:00 2001 From: Feiyang Li Date: Wed, 21 Jan 2026 10:56:23 +0800 Subject: [PATCH 2/4] 1 --- src/iceberg/json_internal.h | 6 ------ src/iceberg/test/json_internal_test.cc | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/iceberg/json_internal.h b/src/iceberg/json_internal.h index 4d32d6896..85a4ed6ca 100644 --- a/src/iceberg/json_internal.h +++ b/src/iceberg/json_internal.h @@ -77,9 +77,6 @@ ICEBERG_EXPORT Result> SortOrderFromJson( /// \brief Deserializes a JSON object into an unbound `SortOrder` object. /// -/// This function parses the provided JSON and creates a `SortOrder` object without -/// binding it to a schema. It does not validate the sort fields against any schema. -/// /// \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. @@ -196,9 +193,6 @@ ICEBERG_EXPORT Result> PartitionSpecFromJson( /// \brief Deserializes a JSON object into an unbound `PartitionSpec` object. /// -/// This function parses the provided JSON and creates a `PartitionSpec` object without -/// binding it to a schema. It does not validate the partition fields against any schema. -/// /// \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. diff --git a/src/iceberg/test/json_internal_test.cc b/src/iceberg/test/json_internal_test.cc index 85a67d06d..d6a171e05 100644 --- a/src/iceberg/test/json_internal_test.cc +++ b/src/iceberg/test/json_internal_test.cc @@ -175,7 +175,7 @@ TEST(JsonInternalTest, PartitionSpec) { EXPECT_EQ(*spec, *parsed_spec_result.value()); } -TEST(JsonInternalTest, SortOrderFromJsonUnbound) { +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); @@ -186,7 +186,7 @@ TEST(JsonInternalTest, SortOrderFromJsonUnbound) { EXPECT_EQ(*sort_order, *parsed); } -TEST(JsonInternalTest, PartitionSpecFromJsonUnbound) { +TEST(JsonInternalTest, PartitionSpecFromJson) { auto identity_transform = Transform::Identity(); ICEBERG_UNWRAP_OR_FAIL( auto spec, From edc422e8c284df32e7e082e129bb2cb055265e4f Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Wed, 21 Jan 2026 12:32:30 +0800 Subject: [PATCH 3/4] Update src/iceberg/json_internal.h --- src/iceberg/json_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iceberg/json_internal.h b/src/iceberg/json_internal.h index 85a4ed6ca..884a7cdfd 100644 --- a/src/iceberg/json_internal.h +++ b/src/iceberg/json_internal.h @@ -75,7 +75,7 @@ ICEBERG_EXPORT nlohmann::json ToJson(const SortOrder& sort_order); ICEBERG_EXPORT Result> SortOrderFromJson( const nlohmann::json& json, const std::shared_ptr& current_schema); -/// \brief Deserializes a JSON object into an unbound `SortOrder` object. +/// \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 From 11a951afb0f5c89223ef941c0c125db721ebbd27 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Wed, 21 Jan 2026 12:46:57 +0800 Subject: [PATCH 4/4] Update src/iceberg/json_internal.h --- src/iceberg/json_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iceberg/json_internal.h b/src/iceberg/json_internal.h index 884a7cdfd..7b09acdbc 100644 --- a/src/iceberg/json_internal.h +++ b/src/iceberg/json_internal.h @@ -191,7 +191,7 @@ ICEBERG_EXPORT Result> PartitionSpecFromJson( const std::shared_ptr& schema, const nlohmann::json& json, int32_t default_spec_id); -/// \brief Deserializes a JSON object into an unbound `PartitionSpec` object. +/// \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