This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 92cbbda refactor: make SortOrder constructor private (#305)
92cbbda is described below
commit 92cbbdac26d02a33df42594402d837819a3230a6
Author: Junwang Zhao <[email protected]>
AuthorDate: Wed Nov 12 10:18:32 2025 +0800
refactor: make SortOrder constructor private (#305)
---
src/iceberg/json_internal.cc | 26 ++++-
src/iceberg/json_internal.h | 3 +-
src/iceberg/sort_order.cc | 9 +-
src/iceberg/sort_order.h | 8 +-
src/iceberg/test/json_internal_test.cc | 20 ++--
src/iceberg/test/metadata_serde_test.cc | 28 +++--
src/iceberg/test/sort_order_test.cc | 183 +++++++++++++++++---------------
7 files changed, 162 insertions(+), 115 deletions(-)
diff --git a/src/iceberg/json_internal.cc b/src/iceberg/json_internal.cc
index 5f45cd9..ebbc5de 100644
--- a/src/iceberg/json_internal.cc
+++ b/src/iceberg/json_internal.cc
@@ -206,6 +206,19 @@ Result<std::unique_ptr<SortField>> SortFieldFromJson(const
nlohmann::json& json)
null_order);
}
+Result<std::unique_ptr<SortOrder>> SortOrderFromJson(
+ const nlohmann::json& json, const std::shared_ptr<Schema>& current_schema)
{
+ 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(*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));
@@ -215,7 +228,7 @@ Result<std::unique_ptr<SortOrder>> 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<SortOrder>(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<Schema>& current_schema,
int32_t& default_sort_order_id,
std::vector<std::shared_ptr<SortOrder>>& 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<nlohmann::json>(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<std::unique_ptr<TableMetadata>>
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 d4741e3..4f9cef8 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<std::unique_ptr<SortOrder>> SortOrderFromJson(
- const nlohmann::json& json);
+ const nlohmann::json& json, const std::shared_ptr<Schema>& current_schema);
/// \brief Convert an Iceberg Schema to JSON.
///
diff --git a/src/iceberg/sort_order.cc b/src/iceberg/sort_order.cc
index 9a7dfe2..f8978a8 100644
--- a/src/iceberg/sort_order.cc
+++ b/src/iceberg/sort_order.cc
@@ -20,6 +20,7 @@
#include "iceberg/sort_order.h"
#include <format>
+#include <memory>
#include <optional>
#include <ranges>
@@ -36,8 +37,8 @@ SortOrder::SortOrder(int32_t order_id, std::vector<SortField>
fields)
: order_id_(order_id), fields_(std::move(fields)) {}
const std::shared_ptr<SortOrder>& SortOrder::Unsorted() {
- static const std::shared_ptr<SortOrder> unsorted =
- std::make_shared<SortOrder>(kUnsortedOrderId, std::vector<SortField>{});
+ static const std::shared_ptr<SortOrder> unsorted =
std::shared_ptr<SortOrder>(
+ new SortOrder(kUnsortedOrderId, std::vector<SortField>{}));
return unsorted;
}
@@ -113,7 +114,7 @@ Result<std::unique_ptr<SortOrder>> SortOrder::Make(const
Schema& schema, int32_t
return InvalidArgument("Sort order must have at least one sort field");
}
- auto sort_order = std::make_unique<SortOrder>(sort_id, std::move(fields));
+ auto sort_order = std::unique_ptr<SortOrder>(new SortOrder(sort_id,
std::move(fields)));
ICEBERG_RETURN_UNEXPECTED(sort_order->Validate(schema));
return sort_order;
}
@@ -128,7 +129,7 @@ Result<std::unique_ptr<SortOrder>> SortOrder::Make(int32_t
sort_id,
return InvalidArgument("Sort order must have at least one sort field");
}
- return std::make_unique<SortOrder>(sort_id, std::move(fields));
+ return std::unique_ptr<SortOrder>(new SortOrder(sort_id, std::move(fields)));
}
} // namespace iceberg
diff --git a/src/iceberg/sort_order.h b/src/iceberg/sort_order.h
index 4a0b11a..85ed0c3 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<SortField> fields);
-
/// \brief Get an unsorted sort order singleton.
static const std::shared_ptr<SortOrder>& Unsorted();
@@ -95,6 +93,12 @@ class ICEBERG_EXPORT SortOrder : public util::Formattable {
std::vector<SortField>
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<SortField> 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 64cddad..64f9c11 100644
--- a/src/iceberg/test/json_internal_test.cc
+++ b/src/iceberg/test/json_internal_test.cc
@@ -49,11 +49,6 @@ Result<std::unique_ptr<SortField>> FromJsonHelper(const
nlohmann::json& json) {
return SortFieldFromJson(json);
}
-template <>
-Result<std::unique_ptr<SortOrder>> FromJsonHelper(const nlohmann::json& json) {
- return SortOrderFromJson(json);
-}
-
template <>
Result<std::unique_ptr<PartitionField>> FromJsonHelper(const nlohmann::json&
json) {
return PartitionFieldFromJson(json);
@@ -107,17 +102,26 @@ TEST(JsonInternalTest, SortField) {
}
TEST(JsonInternalTest, SortOrder) {
+ auto schema = std::make_shared<Schema>(
+ std::vector<SchemaField>{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 9dac1e3..cb1cd80 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>{PartitionField(/*source_id=*/1,
/*field_id=*/1000, "x",
Transform::Identity())});
- auto expected_sort_order = std::make_shared<SortOrder>(
- /*order_id=*/3,
- std::vector<SortField>{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>{
+ 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<SortOrder>(std::move(sort_order));
auto expected_snapshot_1 = std::make_shared<Snapshot>(Snapshot{
.snapshot_id = 3051729675574597004,
@@ -228,13 +233,18 @@ TEST(MetadataSerdeTest, DeserializeV2ValidMinimal) {
std::vector<PartitionField>{PartitionField(/*source_id=*/1,
/*field_id=*/1000, "x",
Transform::Identity())});
- auto expected_sort_order = std::make_shared<SortOrder>(
- /*order_id=*/3, std::vector<SortField>{
+ ICEBERG_UNWRAP_OR_FAIL(
+ auto sort_order,
+ SortOrder::Make(*expected_schema,
+ /*order_id=*/3,
+ std::vector<SortField>{
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<SortOrder>(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 86bd525..d2bef58 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<SchemaField>(1, "x", int32(), true);
field2_ = std::make_unique<SchemaField>(2, "y", string(), true);
- field3_ = std::make_unique<SchemaField>(3, "time", timestamp(), true);
+ field3_ = std::make_unique<SchemaField>(5, "ts", iceberg::timestamp(),
true);
+ field4_ = std::make_unique<SchemaField>(7, "bar", iceberg::string(), true);
schema_ = std::make_unique<Schema>(
- std::vector<SchemaField>{*field1_, *field2_, *field3_}, 1);
+ std::vector<SchemaField>{*field1_, *field2_, *field3_, *field4_}, 1);
sort_field1_ = std::make_unique<SortField>(
1, Transform::Identity(), SortDirection::kAscending,
NullOrder::kFirst);
sort_field2_ = std::make_unique<SortField>(
2, Transform::Bucket(10), SortDirection::kDescending,
NullOrder::kLast);
sort_field3_ = std::make_unique<SortField>(
- 3, Transform::Day(), SortDirection::kAscending, NullOrder::kFirst);
+ 5, Transform::Day(), SortDirection::kAscending, NullOrder::kFirst);
}
std::unique_ptr<Schema> schema_;
std::unique_ptr<SchemaField> field1_;
std::unique_ptr<SchemaField> field2_;
std::unique_ptr<SchemaField> field3_;
+ std::unique_ptr<SchemaField> field4_;
std::unique_ptr<SortField> sort_field1_;
std::unique_ptr<SortField> sort_field2_;
std::unique_ptr<SortField> 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<const SortField> 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<const SortField> 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<SortField>{*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<SortField>{});
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<SortField>{*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<SortField>{}));
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<SortField>{*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<SchemaField>(
4, "struct_field",
std::make_shared<StructType>(std::vector<SchemaField>{
@@ -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<SortField>{*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<SortField>{*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"));
