diff --git a/src/iceberg/json_internal.cc b/src/iceberg/json_internal.cc index 5f45cd938..ebbc5de65 100644 --- a/src/iceberg/json_internal.cc +++ b/src/iceberg/json_internal.cc @@ -206,6 +206,19 @@ Result> SortFieldFromJson(const nlohmann::json& json) null_order); } +Result> SortOrderFromJson( + const nlohmann::json& json, const std::shared_ptr& current_schema) { + 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(*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)); @@ -215,7 +228,7 @@ Result> SortOrderFromJson(const nlohmann::json& json) ICEBERG_ASSIGN_OR_RAISE(auto sort_field, SortFieldFromJson(field_json)); sort_fields.push_back(std::move(*sort_field)); } - return std::make_unique(order_id, std::move(sort_fields)); + return SortOrder::Make(order_id, std::move(sort_fields)); } nlohmann::json ToJson(const SchemaField& field) { @@ -919,9 +932,11 @@ Status ParsePartitionSpecs(const nlohmann::json& json, int8_t format_version, /// /// \param[in] json The JSON object to parse. /// \param[in] format_version The format version of the table. +/// \param[in] current_schema The current schema. /// \param[out] default_sort_order_id The default sort order ID. /// \param[out] sort_orders The list of sort orders. Status ParseSortOrders(const nlohmann::json& json, int8_t format_version, + const std::shared_ptr& current_schema, int32_t& default_sort_order_id, std::vector>& sort_orders) { if (json.contains(kSortOrders)) { @@ -930,7 +945,8 @@ Status ParseSortOrders(const nlohmann::json& json, int8_t format_version, ICEBERG_ASSIGN_OR_RAISE(auto sort_order_array, GetJsonValue(json, kSortOrders)); for (const auto& sort_order_json : sort_order_array) { - ICEBERG_ASSIGN_OR_RAISE(auto sort_order, SortOrderFromJson(sort_order_json)); + ICEBERG_ASSIGN_OR_RAISE(auto sort_order, + SortOrderFromJson(sort_order_json, current_schema)); sort_orders.push_back(std::move(sort_order)); } } else { @@ -1005,9 +1021,9 @@ Result> TableMetadataFromJson(const nlohmann::jso } } - ICEBERG_RETURN_UNEXPECTED(ParseSortOrders(json, table_metadata->format_version, - table_metadata->default_sort_order_id, - table_metadata->sort_orders)); + ICEBERG_RETURN_UNEXPECTED(ParseSortOrders( + json, table_metadata->format_version, current_schema, + table_metadata->default_sort_order_id, table_metadata->sort_orders)); if (json.contains(kProperties)) { ICEBERG_ASSIGN_OR_RAISE(table_metadata->properties, FromJsonMap(json, kProperties)); diff --git a/src/iceberg/json_internal.h b/src/iceberg/json_internal.h index d4741e3b8..4f9cef8e8 100644 --- a/src/iceberg/json_internal.h +++ b/src/iceberg/json_internal.h @@ -69,10 +69,11 @@ ICEBERG_EXPORT nlohmann::json ToJson(const SortOrder& sort_order); /// Each `SortField` will be parsed using the `SortFieldFromJson` function. /// /// \param json The JSON object representing a `SortOrder`. +/// \param current_schema The current schema associated with the sort order. /// \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); + const nlohmann::json& json, const std::shared_ptr& current_schema); /// \brief Convert an Iceberg Schema to JSON. /// diff --git a/src/iceberg/sort_order.cc b/src/iceberg/sort_order.cc index 9a7dfe299..f8978a80e 100644 --- a/src/iceberg/sort_order.cc +++ b/src/iceberg/sort_order.cc @@ -20,6 +20,7 @@ #include "iceberg/sort_order.h" #include +#include #include #include @@ -36,8 +37,8 @@ SortOrder::SortOrder(int32_t order_id, std::vector fields) : order_id_(order_id), fields_(std::move(fields)) {} const std::shared_ptr& SortOrder::Unsorted() { - static const std::shared_ptr unsorted = - std::make_shared(kUnsortedOrderId, std::vector{}); + static const std::shared_ptr unsorted = std::shared_ptr( + new SortOrder(kUnsortedOrderId, std::vector{})); return unsorted; } @@ -113,7 +114,7 @@ Result> SortOrder::Make(const Schema& schema, int32_t return InvalidArgument("Sort order must have at least one sort field"); } - auto sort_order = std::make_unique(sort_id, std::move(fields)); + auto sort_order = std::unique_ptr(new SortOrder(sort_id, std::move(fields))); ICEBERG_RETURN_UNEXPECTED(sort_order->Validate(schema)); return sort_order; } @@ -128,7 +129,7 @@ Result> SortOrder::Make(int32_t sort_id, return InvalidArgument("Sort order must have at least one sort field"); } - return std::make_unique(sort_id, std::move(fields)); + return std::unique_ptr(new SortOrder(sort_id, std::move(fields))); } } // namespace iceberg diff --git a/src/iceberg/sort_order.h b/src/iceberg/sort_order.h index 4a0b11a92..85ed0c3de 100644 --- a/src/iceberg/sort_order.h +++ b/src/iceberg/sort_order.h @@ -41,8 +41,6 @@ class ICEBERG_EXPORT SortOrder : public util::Formattable { static constexpr int32_t kUnsortedOrderId = 0; static constexpr int32_t kInitialSortOrderId = 1; - SortOrder(int32_t order_id, std::vector fields); - /// \brief Get an unsorted sort order singleton. static const std::shared_ptr& Unsorted(); @@ -95,6 +93,12 @@ class ICEBERG_EXPORT SortOrder : public util::Formattable { std::vector fields); private: + /// \brief Constructs a SortOrder instance. + /// \param order_id The sort order id. + /// \param fields The sort fields. + /// \note Use the static Make methods to create SortOrder instances. + SortOrder(int32_t order_id, std::vector fields); + /// \brief Compare two sort orders for equality. bool Equals(const SortOrder& other) const; diff --git a/src/iceberg/test/json_internal_test.cc b/src/iceberg/test/json_internal_test.cc index 64cddad9e..64f9c1125 100644 --- a/src/iceberg/test/json_internal_test.cc +++ b/src/iceberg/test/json_internal_test.cc @@ -49,11 +49,6 @@ Result> FromJsonHelper(const nlohmann::json& json) { return SortFieldFromJson(json); } -template <> -Result> FromJsonHelper(const nlohmann::json& json) { - return SortOrderFromJson(json); -} - template <> Result> FromJsonHelper(const nlohmann::json& json) { return PartitionFieldFromJson(json); @@ -107,17 +102,26 @@ TEST(JsonInternalTest, SortField) { } TEST(JsonInternalTest, SortOrder) { + auto schema = std::make_shared( + std::vector{SchemaField(5, "region", iceberg::string(), false), + SchemaField(7, "ts", iceberg::int64(), false)}, + /*schema_id=*/100); auto identity_transform = Transform::Identity(); SortField st_ts(5, identity_transform, SortDirection::kAscending, NullOrder::kFirst); SortField st_bar(7, identity_transform, SortDirection::kDescending, NullOrder::kLast); - SortOrder sort_order(100, {st_ts, st_bar}); - + ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(100, {st_ts, st_bar})); + EXPECT_TRUE(sort_order->Validate(*schema)); nlohmann::json expected_sort_order = R"({"order-id":100,"fields":[ {"transform":"identity","source-id":5,"direction":"asc","null-order":"nulls-first"}, {"transform":"identity","source-id":7,"direction":"desc","null-order":"nulls-last"}]})"_json; - TestJsonConversion(sort_order, expected_sort_order); + auto json = ToJson(*sort_order); + EXPECT_EQ(expected_sort_order, json) << "JSON conversion mismatch."; + + // Specialize FromJson based on type (T) + ICEBERG_UNWRAP_OR_FAIL(auto obj_ex, SortOrderFromJson(expected_sort_order, schema)); + EXPECT_EQ(*sort_order, *obj_ex) << "Deserialized object mismatch."; } TEST(JsonInternalTest, PartitionField) { diff --git a/src/iceberg/test/metadata_serde_test.cc b/src/iceberg/test/metadata_serde_test.cc index 9dac1e324..cb1cd8005 100644 --- a/src/iceberg/test/metadata_serde_test.cc +++ b/src/iceberg/test/metadata_serde_test.cc @@ -148,12 +148,17 @@ TEST(MetadataSerdeTest, DeserializeV2Valid) { std::vector{PartitionField(/*source_id=*/1, /*field_id=*/1000, "x", Transform::Identity())}); - auto expected_sort_order = std::make_shared( - /*order_id=*/3, - std::vector{SortField(/*source_id=*/2, Transform::Identity(), - SortDirection::kAscending, NullOrder::kFirst), - SortField(/*source_id=*/3, Transform::Bucket(4), - SortDirection::kDescending, NullOrder::kLast)}); + ICEBERG_UNWRAP_OR_FAIL( + auto sort_order, + SortOrder::Make(*expected_schema_2, + /*order_id=*/3, + std::vector{ + SortField(/*source_id=*/2, Transform::Identity(), + SortDirection::kAscending, NullOrder::kFirst), + SortField(/*source_id=*/3, Transform::Bucket(4), + SortDirection::kDescending, NullOrder::kLast)})); + + auto expected_sort_order = std::shared_ptr(std::move(sort_order)); auto expected_snapshot_1 = std::make_shared(Snapshot{ .snapshot_id = 3051729675574597004, @@ -228,13 +233,18 @@ TEST(MetadataSerdeTest, DeserializeV2ValidMinimal) { std::vector{PartitionField(/*source_id=*/1, /*field_id=*/1000, "x", Transform::Identity())}); - auto expected_sort_order = std::make_shared( - /*order_id=*/3, std::vector{ + ICEBERG_UNWRAP_OR_FAIL( + auto sort_order, + SortOrder::Make(*expected_schema, + /*order_id=*/3, + std::vector{ SortField(/*source_id=*/2, Transform::Identity(), SortDirection::kAscending, NullOrder::kFirst), SortField(/*source_id=*/3, Transform::Bucket(4), SortDirection::kDescending, NullOrder::kLast), - }); + })); + + auto expected_sort_order = std::shared_ptr(std::move(sort_order)); TableMetadata expected{ .format_version = 2, diff --git a/src/iceberg/test/sort_order_test.cc b/src/iceberg/test/sort_order_test.cc index 86bd52567..d2bef580a 100644 --- a/src/iceberg/test/sort_order_test.cc +++ b/src/iceberg/test/sort_order_test.cc @@ -32,63 +32,59 @@ namespace iceberg { -class SortOrderMakeTest : public ::testing::Test { +class SortOrderTest : public ::testing::Test { protected: void SetUp() override { field1_ = std::make_unique(1, "x", int32(), true); field2_ = std::make_unique(2, "y", string(), true); - field3_ = std::make_unique(3, "time", timestamp(), true); + field3_ = std::make_unique(5, "ts", iceberg::timestamp(), true); + field4_ = std::make_unique(7, "bar", iceberg::string(), true); schema_ = std::make_unique( - std::vector{*field1_, *field2_, *field3_}, 1); + std::vector{*field1_, *field2_, *field3_, *field4_}, 1); sort_field1_ = std::make_unique( 1, Transform::Identity(), SortDirection::kAscending, NullOrder::kFirst); sort_field2_ = std::make_unique( 2, Transform::Bucket(10), SortDirection::kDescending, NullOrder::kLast); sort_field3_ = std::make_unique( - 3, Transform::Day(), SortDirection::kAscending, NullOrder::kFirst); + 5, Transform::Day(), SortDirection::kAscending, NullOrder::kFirst); } std::unique_ptr schema_; std::unique_ptr field1_; std::unique_ptr field2_; std::unique_ptr field3_; + std::unique_ptr field4_; std::unique_ptr sort_field1_; std::unique_ptr sort_field2_; std::unique_ptr sort_field3_; }; -TEST(SortOrderTest, Basics) { - { - SchemaField field1(5, "ts", iceberg::timestamp(), true); - SchemaField field2(7, "bar", iceberg::string(), true); - - auto identity_transform = Transform::Identity(); - SortField st_field1(5, identity_transform, SortDirection::kAscending, - NullOrder::kFirst); - SortField st_field2(7, identity_transform, SortDirection::kDescending, - NullOrder::kFirst); - SortOrder sort_order(100, {st_field1, st_field2}); - ASSERT_EQ(sort_order, sort_order); - std::span fields = sort_order.fields(); - ASSERT_EQ(2, fields.size()); - ASSERT_EQ(st_field1, fields[0]); - ASSERT_EQ(st_field2, fields[1]); - auto sort_order_str = - "[\n" - " identity(5) asc nulls-first\n" - " identity(7) desc nulls-first\n" - "]"; - EXPECT_EQ(sort_order.ToString(), sort_order_str); - EXPECT_EQ(std::format("{}", sort_order), sort_order_str); - } +TEST_F(SortOrderTest, Basics) { + auto identity_transform = Transform::Identity(); + SortField st_field1(5, identity_transform, SortDirection::kAscending, + NullOrder::kFirst); + SortField st_field2(7, identity_transform, SortDirection::kDescending, + NullOrder::kFirst); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order, + SortOrder::Make(*schema_, 100, {st_field1, st_field2})); + ASSERT_EQ(*sort_order, *sort_order); + std::span fields = sort_order->fields(); + ASSERT_EQ(2, fields.size()); + ASSERT_EQ(st_field1, fields[0]); + ASSERT_EQ(st_field2, fields[1]); + auto sort_order_str = + "[\n" + " identity(5) asc nulls-first\n" + " identity(7) desc nulls-first\n" + "]"; + EXPECT_EQ(sort_order->ToString(), sort_order_str); + EXPECT_EQ(std::format("{}", *sort_order), sort_order_str); } -TEST(SortOrderTest, Equality) { - SchemaField field1(5, "ts", iceberg::timestamp(), true); - SchemaField field2(7, "bar", iceberg::string(), true); +TEST_F(SortOrderTest, Equality) { auto bucket_transform = Transform::Bucket(8); auto identity_transform = Transform::Identity(); SortField st_field1(5, identity_transform, SortDirection::kAscending, @@ -96,43 +92,45 @@ TEST(SortOrderTest, Equality) { SortField st_field2(7, identity_transform, SortDirection::kDescending, NullOrder::kFirst); SortField st_field3(7, bucket_transform, SortDirection::kAscending, NullOrder::kFirst); - SortOrder sort_order1(100, {st_field1, st_field2}); - SortOrder sort_order2(100, {st_field2, st_field3}); - SortOrder sort_order3(100, {st_field1, st_field3}); - SortOrder sort_order4(101, {st_field1, st_field2}); - SortOrder sort_order5(100, {st_field2, st_field1}); - - ASSERT_EQ(sort_order1, sort_order1); - ASSERT_NE(sort_order1, sort_order2); - ASSERT_NE(sort_order2, sort_order1); - ASSERT_NE(sort_order1, sort_order3); - ASSERT_NE(sort_order3, sort_order1); - ASSERT_NE(sort_order1, sort_order4); - ASSERT_NE(sort_order4, sort_order1); - ASSERT_NE(sort_order1, sort_order5); - ASSERT_NE(sort_order5, sort_order1); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order1, + SortOrder::Make(*schema_, 100, {st_field1, st_field2})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order2, + SortOrder::Make(*schema_, 100, {st_field2, st_field3})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order3, + SortOrder::Make(*schema_, 100, {st_field1, st_field3})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order4, + SortOrder::Make(*schema_, 101, {st_field1, st_field2})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order5, + SortOrder::Make(*schema_, 100, {st_field2, st_field1})); + + ASSERT_EQ(*sort_order1, *sort_order1); + ASSERT_NE(*sort_order1, *sort_order2); + ASSERT_NE(*sort_order2, *sort_order1); + ASSERT_NE(*sort_order1, *sort_order3); + ASSERT_NE(*sort_order3, *sort_order1); + ASSERT_NE(*sort_order1, *sort_order4); + ASSERT_NE(*sort_order4, *sort_order1); + ASSERT_NE(*sort_order1, *sort_order5); + ASSERT_NE(*sort_order5, *sort_order1); } -TEST(SortOrderTest, IsUnsorted) { +TEST_F(SortOrderTest, IsUnsorted) { auto unsorted = SortOrder::Unsorted(); EXPECT_TRUE(unsorted->is_unsorted()); EXPECT_FALSE(unsorted->is_sorted()); } -TEST(SortOrderTest, IsSorted) { - SchemaField field1(5, "ts", iceberg::timestamp(), true); +TEST_F(SortOrderTest, IsSorted) { auto identity_transform = Transform::Identity(); SortField st_field1(5, identity_transform, SortDirection::kAscending, NullOrder::kFirst); - SortOrder sorted_order(100, {st_field1}); + ICEBERG_UNWRAP_OR_FAIL(auto sorted_order, SortOrder::Make(*schema_, 100, {st_field1})); - EXPECT_TRUE(sorted_order.is_sorted()); - EXPECT_FALSE(sorted_order.is_unsorted()); + EXPECT_TRUE(sorted_order->is_sorted()); + EXPECT_FALSE(sorted_order->is_unsorted()); } -TEST(SortOrderTest, Satisfies) { - SchemaField field1(5, "ts", iceberg::timestamp(), true); - SchemaField field2(7, "bar", iceberg::string(), true); +TEST_F(SortOrderTest, Satisfies) { auto identity_transform = Transform::Identity(); auto bucket_transform = Transform::Bucket(8); @@ -142,42 +140,44 @@ TEST(SortOrderTest, Satisfies) { NullOrder::kFirst); SortField st_field3(7, bucket_transform, SortDirection::kAscending, NullOrder::kFirst); - SortOrder sort_order1(100, {st_field1, st_field2}); - SortOrder sort_order2(101, {st_field1}); - SortOrder sort_order3(102, {st_field1, st_field3}); - SortOrder sort_order4(104, {st_field2}); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order1, + SortOrder::Make(*schema_, 100, {st_field1, st_field2})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order2, SortOrder::Make(*schema_, 101, {st_field1})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order3, + SortOrder::Make(*schema_, 102, {st_field1, st_field3})); + ICEBERG_UNWRAP_OR_FAIL(auto sort_order4, SortOrder::Make(*schema_, 104, {st_field2})); auto unsorted = SortOrder::Unsorted(); // Any order satisfies an unsorted order, including unsorted itself EXPECT_TRUE(unsorted->Satisfies(*unsorted)); - EXPECT_TRUE(sort_order1.Satisfies(*unsorted)); - EXPECT_TRUE(sort_order2.Satisfies(*unsorted)); - EXPECT_TRUE(sort_order3.Satisfies(*unsorted)); + EXPECT_TRUE(sort_order1->Satisfies(*unsorted)); + EXPECT_TRUE(sort_order2->Satisfies(*unsorted)); + EXPECT_TRUE(sort_order3->Satisfies(*unsorted)); // Unsorted does not satisfy any sorted order - EXPECT_FALSE(unsorted->Satisfies(sort_order1)); - EXPECT_FALSE(unsorted->Satisfies(sort_order2)); - EXPECT_FALSE(unsorted->Satisfies(sort_order3)); + EXPECT_FALSE(unsorted->Satisfies(*sort_order1)); + EXPECT_FALSE(unsorted->Satisfies(*sort_order2)); + EXPECT_FALSE(unsorted->Satisfies(*sort_order3)); // A sort order satisfies itself - EXPECT_TRUE(sort_order1.Satisfies(sort_order1)); - EXPECT_TRUE(sort_order2.Satisfies(sort_order2)); - EXPECT_TRUE(sort_order3.Satisfies(sort_order3)); + EXPECT_TRUE(sort_order1->Satisfies(*sort_order1)); + EXPECT_TRUE(sort_order2->Satisfies(*sort_order2)); + EXPECT_TRUE(sort_order3->Satisfies(*sort_order3)); // A sort order with more fields satisfy one with fewer fields - EXPECT_TRUE(sort_order1.Satisfies(sort_order2)); - EXPECT_TRUE(sort_order3.Satisfies(sort_order2)); + EXPECT_TRUE(sort_order1->Satisfies(*sort_order2)); + EXPECT_TRUE(sort_order3->Satisfies(*sort_order2)); // A sort order does not satisfy one with more fields - EXPECT_FALSE(sort_order2.Satisfies(sort_order1)); - EXPECT_FALSE(sort_order2.Satisfies(sort_order3)); + EXPECT_FALSE(sort_order2->Satisfies(*sort_order1)); + EXPECT_FALSE(sort_order2->Satisfies(*sort_order3)); - // A sort order does not satify one with different fields - EXPECT_FALSE(sort_order4.Satisfies(sort_order2)); - EXPECT_FALSE(sort_order2.Satisfies(sort_order4)); + // A sort order does not satisfy one with different fields + EXPECT_FALSE(sort_order4->Satisfies(*sort_order2)); + EXPECT_FALSE(sort_order2->Satisfies(*sort_order4)); } -TEST_F(SortOrderMakeTest, MakeValidSortOrder) { +TEST_F(SortOrderTest, MakeValidSortOrder) { ICEBERG_UNWRAP_OR_FAIL( auto sort_order, SortOrder::Make(*schema_, 1, std::vector{*sort_field1_, *sort_field2_})); @@ -189,14 +189,14 @@ TEST_F(SortOrderMakeTest, MakeValidSortOrder) { EXPECT_EQ(sort_order->fields()[1], *sort_field2_); } -TEST_F(SortOrderMakeTest, MakeInvalidSortOrderEmptyFields) { +TEST_F(SortOrderTest, MakeInvalidSortOrderEmptyFields) { auto sort_order = SortOrder::Make(*schema_, 1, std::vector{}); EXPECT_THAT(sort_order, IsError(ErrorKind::kInvalidArgument)); EXPECT_THAT(sort_order, HasErrorMessage("Sort order must have at least one sort field")); } -TEST_F(SortOrderMakeTest, MakeInvalidSortOrderUnsortedId) { +TEST_F(SortOrderTest, MakeInvalidSortOrderUnsortedId) { auto sort_order = SortOrder::Make(*schema_, SortOrder::kUnsortedOrderId, std::vector{*sort_field1_}); EXPECT_THAT(sort_order, IsError(ErrorKind::kInvalidArgument)); @@ -205,7 +205,7 @@ TEST_F(SortOrderMakeTest, MakeInvalidSortOrderUnsortedId) { SortOrder::kUnsortedOrderId))); } -TEST_F(SortOrderMakeTest, MakeValidUnsortedSortOrder) { +TEST_F(SortOrderTest, MakeValidUnsortedSortOrder) { ICEBERG_UNWRAP_OR_FAIL(auto sort_order, SortOrder::Make(SortOrder::kUnsortedOrderId, std::vector{})); ASSERT_NE(sort_order, nullptr); @@ -214,7 +214,18 @@ TEST_F(SortOrderMakeTest, MakeValidUnsortedSortOrder) { EXPECT_EQ(sort_order->fields().size(), 0); } -TEST_F(SortOrderMakeTest, MakeInvalidSortOrderNonPrimitiveField) { +TEST_F(SortOrderTest, MakeInvalidSortOrderTransformCannotApply) { + // Day transform cannot be applied to string type + SortField sort_field_invalid(2, Transform::Day(), SortDirection::kAscending, + NullOrder::kFirst); + + auto sort_order = SortOrder::Make( + *schema_, 1, std::vector{*sort_field1_, sort_field_invalid}); + EXPECT_THAT(sort_order, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(sort_order, HasErrorMessage("Invalid source type")); +} + +TEST_F(SortOrderTest, MakeInvalidSortOrderNonPrimitiveField) { auto struct_field = std::make_unique( 4, "struct_field", std::make_shared(std::vector{ @@ -234,7 +245,7 @@ TEST_F(SortOrderMakeTest, MakeInvalidSortOrderNonPrimitiveField) { EXPECT_THAT(sort_order, HasErrorMessage("Invalid source type")); } -TEST_F(SortOrderMakeTest, MakeInvalidSortOrderFieldNotInSchema) { +TEST_F(SortOrderTest, MakeInvalidSortOrderFieldNotInSchema) { SortField sort_field_invalid(999, Transform::Identity(), SortDirection::kAscending, NullOrder::kFirst); @@ -244,14 +255,14 @@ TEST_F(SortOrderMakeTest, MakeInvalidSortOrderFieldNotInSchema) { EXPECT_THAT(sort_order, HasErrorMessage("Cannot find source column for sort field")); } -TEST_F(SortOrderMakeTest, MakeUnboundSortOrder) { +TEST_F(SortOrderTest, MakeUnboundSortOrder) { SortField sort_field_invalid(999, Transform::Identity(), SortDirection::kAscending, NullOrder::kFirst); - auto sort_order = - SortOrder::Make(1, std::vector{*sort_field1_, sort_field_invalid}); - ASSERT_THAT(sort_order, IsOk()); - auto validate_status = sort_order.value()->Validate(*schema_); + ICEBERG_UNWRAP_OR_FAIL( + auto sort_order, + SortOrder::Make(1, std::vector{*sort_field1_, sort_field_invalid})); + auto validate_status = sort_order->Validate(*schema_); EXPECT_THAT(validate_status, IsError(ErrorKind::kInvalidArgument)); EXPECT_THAT(validate_status, HasErrorMessage("Cannot find source column for sort field"));