Skip to content

Commit 1d45a6e

Browse files
HeartLinkedHeartLinked
committed
1
1 parent 0635b27 commit 1d45a6e

2 files changed

Lines changed: 27 additions & 65 deletions

File tree

‎src/iceberg/test/update_properties_test.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ namespace iceberg {
2727
class UpdatePropertiesTest : public UpdateTestBase {};
2828

2929
TEST_F(UpdatePropertiesTest, EmptyUpdate) {
30-
// commit an empty update, should succeed
3130
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
3231
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
3332
EXPECT_THAT(result.updates.empty(), true);

‎src/iceberg/test/update_sort_order_test.cc‎

Lines changed: 27 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -36,132 +36,107 @@
3636

3737
namespace iceberg {
3838

39-
class UpdateSortOrderTest : public UpdateTestBase {};
39+
class UpdateSortOrderTest : public UpdateTestBase {
40+
protected:
41+
// Helper function to apply update and verify the resulting sort order
42+
void ApplyAndExpectSortOrder(UpdateSortOrder* update,
43+
std::vector<SortField> expected_fields) {
44+
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
45+
ICEBERG_UNWRAP_OR_FAIL(
46+
auto expected_sort_order,
47+
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
48+
EXPECT_EQ(*result.sort_order, *expected_sort_order);
49+
}
50+
};
51+
52+
TEST_F(UpdateSortOrderTest, EmptySortOrder) {
53+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
54+
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
55+
// Should succeed with an unsorted order
56+
EXPECT_TRUE(result.sort_order->fields().empty());
57+
}
4058

4159
TEST_F(UpdateSortOrderTest, AddSingleSortFieldAscending) {
4260
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
4361
auto term = Expressions::Transform("x", Transform::Identity());
44-
4562
update->AddSortField(term, SortDirection::kAscending, NullOrder::kFirst);
4663

47-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
48-
EXPECT_FALSE(result.sort_order->is_unsorted());
49-
5064
std::vector<SortField> expected_fields;
5165
expected_fields.emplace_back(1, Transform::Identity(), SortDirection::kAscending,
5266
NullOrder::kFirst);
53-
ICEBERG_UNWRAP_OR_FAIL(
54-
auto expected_sort_order,
55-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
56-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
67+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
5768
}
5869

5970
TEST_F(UpdateSortOrderTest, AddSingleSortFieldDescending) {
6071
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
6172
auto term = Expressions::Transform("y", Transform::Identity());
62-
6373
update->AddSortField(term, SortDirection::kDescending, NullOrder::kLast);
6474

65-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
66-
6775
std::vector<SortField> expected_fields;
6876
expected_fields.emplace_back(2, Transform::Identity(), SortDirection::kDescending,
6977
NullOrder::kLast);
70-
ICEBERG_UNWRAP_OR_FAIL(
71-
auto expected_sort_order,
72-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
73-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
78+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
7479
}
7580

7681
TEST_F(UpdateSortOrderTest, AddMultipleSortFields) {
7782
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
7883
auto term1 = Expressions::Transform("y", Transform::Identity());
7984
auto term2 = Expressions::Transform("x", Transform::Identity());
80-
8185
update->AddSortField(term1, SortDirection::kAscending, NullOrder::kFirst)
8286
.AddSortField(term2, SortDirection::kDescending, NullOrder::kLast);
8387

84-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
85-
8688
std::vector<SortField> expected_fields;
8789
expected_fields.emplace_back(2, Transform::Identity(), SortDirection::kAscending,
8890
NullOrder::kFirst);
8991
expected_fields.emplace_back(1, Transform::Identity(), SortDirection::kDescending,
9092
NullOrder::kLast);
91-
ICEBERG_UNWRAP_OR_FAIL(
92-
auto expected_sort_order,
93-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
94-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
93+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
9594
}
9695

9796
TEST_F(UpdateSortOrderTest, AddSortFieldWithNamedReference) {
9897
// Test that we can directly use NamedReference (kReference) which is treated as
9998
// identity
10099
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
101100
auto ref = Expressions::Ref("x");
102-
103101
update->AddSortField(ref, SortDirection::kAscending, NullOrder::kFirst);
104102

105-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
106-
107103
std::vector<SortField> expected_fields;
108104
expected_fields.emplace_back(1, Transform::Identity(), SortDirection::kAscending,
109105
NullOrder::kFirst);
110-
ICEBERG_UNWRAP_OR_FAIL(
111-
auto expected_sort_order,
112-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
113-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
106+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
114107
}
115108

116109
TEST_F(UpdateSortOrderTest, AddSortFieldByName) {
117110
// Test the convenience method for adding sort field by name
118111
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
119-
120112
update->AddSortFieldByName("x", SortDirection::kAscending, NullOrder::kFirst);
121113

122-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
123-
124114
std::vector<SortField> expected_fields;
125115
expected_fields.emplace_back(1, Transform::Identity(), SortDirection::kAscending,
126116
NullOrder::kFirst);
127-
ICEBERG_UNWRAP_OR_FAIL(
128-
auto expected_sort_order,
129-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
130-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
117+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
131118
}
132119

133120
TEST_F(UpdateSortOrderTest, AddSortFieldWithTruncateTransform) {
134121
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
135122
auto term = Expressions::Truncate("x", 10);
136-
137123
update->AddSortField(term, SortDirection::kAscending, NullOrder::kFirst);
138124

139-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
140-
141125
std::vector<SortField> expected_fields;
142126
expected_fields.emplace_back(1, Transform::Truncate(10), SortDirection::kAscending,
143127
NullOrder::kFirst);
144-
ICEBERG_UNWRAP_OR_FAIL(
145-
auto expected_sort_order,
146-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
147-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
128+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
148129
}
149130

150131
TEST_F(UpdateSortOrderTest, AddSortFieldWithBucketTransform) {
151132
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
152133
auto term = Expressions::Bucket("y", 10);
153-
154134
update->AddSortField(term, SortDirection::kDescending, NullOrder::kLast);
155135

156-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
157-
158136
std::vector<SortField> expected_fields;
159137
expected_fields.emplace_back(2, Transform::Bucket(10), SortDirection::kDescending,
160138
NullOrder::kLast);
161-
ICEBERG_UNWRAP_OR_FAIL(
162-
auto expected_sort_order,
163-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
164-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
139+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
165140
}
166141

167142
TEST_F(UpdateSortOrderTest, AddSortFieldNullTerm) {
@@ -219,19 +194,14 @@ TEST_F(UpdateSortOrderTest, CaseSensitiveFalse) {
219194
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
220195
auto ref = NamedReference::Make("X").value(); // Uppercase
221196
auto term = UnboundTransform::Make(std::move(ref), Transform::Identity()).value();
222-
223197
update->CaseSensitive(false).AddSortField(std::move(term), SortDirection::kAscending,
224198
NullOrder::kFirst);
225199

226-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
227200
// Should succeed because case-insensitive matching
228201
std::vector<SortField> expected_fields;
229202
expected_fields.emplace_back(1, Transform::Identity(), SortDirection::kAscending,
230203
NullOrder::kFirst);
231-
ICEBERG_UNWRAP_OR_FAIL(
232-
auto expected_sort_order,
233-
SortOrder::Make(result.sort_order->order_id(), std::move(expected_fields)));
234-
EXPECT_EQ(*result.sort_order, *expected_sort_order);
204+
ApplyAndExpectSortOrder(update.get(), std::move(expected_fields));
235205
}
236206

237207
TEST_F(UpdateSortOrderTest, CommitSuccess) {
@@ -263,11 +233,4 @@ TEST_F(UpdateSortOrderTest, CommitSuccess) {
263233
EXPECT_EQ(*sort_order, *expected_sort_order);
264234
}
265235

266-
TEST_F(UpdateSortOrderTest, EmptySortOrder) {
267-
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSortOrder());
268-
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
269-
// Should succeed with an unsorted order
270-
EXPECT_TRUE(result.sort_order->fields().empty());
271-
}
272-
273236
} // namespace iceberg

0 commit comments

Comments
 (0)