wgtmac commented on code in PR #518:
URL: https://github.com/apache/iceberg-cpp/pull/518#discussion_r2710750459
##########
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) {
Review Comment:
```suggestion
TEST(JsonInternalTest, PartitionSpecFromJson) {
```
##########
src/iceberg/json_internal.h:
##########
@@ -75,6 +75,17 @@ 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 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.
+///
Review Comment:
```suggestion
/// \brief Deserializes a JSON object into a `SortOrder` object.
///
```
##########
src/iceberg/json_internal.h:
##########
@@ -75,6 +75,17 @@ 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 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.
+///
Review Comment:
I'm hesitant to say this is an unbound object because we don't officially
support `UnboundSortOrder` like the Java does. Users can call
SortOrder::Validate by themselves against a schema on the deserialized value.
##########
src/iceberg/json_internal.h:
##########
@@ -183,6 +194,17 @@ 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 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.
+///
Review Comment:
```suggestion
/// \brief Deserializes a JSON object into a `PartitionSpec` object.
///
```
##########
src/iceberg/json_internal.h:
##########
@@ -183,6 +194,17 @@ 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 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.
+///
Review Comment:
ditto
##########
src/iceberg/test/json_internal_test.cc:
##########
@@ -175,6 +175,29 @@ TEST(JsonInternalTest, PartitionSpec) {
EXPECT_EQ(*spec, *parsed_spec_result.value());
}
+TEST(JsonInternalTest, SortOrderFromJsonUnbound) {
Review Comment:
```suggestion
TEST(JsonInternalTest, SortOrderFromJson) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]