Skip to content

Commit c12f415

Browse files
HeartLinkedHeartLinked
committed
1
1 parent 56b0c03 commit c12f415

File tree

3 files changed

+122
-24
lines changed

3 files changed

+122
-24
lines changed

‎src/iceberg/table_metadata.cc‎

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -336,22 +336,6 @@ struct TableMetadataBuilder::Impl {
336336
sort_orders_by_id.emplace(order->order_id(), order);
337337
}
338338
}
339-
340-
int32_t ReuseOrCreateNewSortOrderId(const SortOrder& new_order) {
341-
if (new_order.is_unsorted()) {
342-
return SortOrder::kUnsortedOrderId;
343-
}
344-
// determine the next order id
345-
int32_t new_order_id = SortOrder::kInitialSortOrderId;
346-
for (const auto& order : metadata.sort_orders) {
347-
if (order->SameOrder(new_order)) {
348-
return order->order_id();
349-
} else if (new_order_id <= order->order_id()) {
350-
new_order_id = order->order_id() + 1;
351-
}
352-
}
353-
return new_order_id;
354-
}
355339
};
356340

357341
TableMetadataBuilder::TableMetadataBuilder(int8_t format_version)
@@ -463,16 +447,38 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveSchemas(
463447

464448
TableMetadataBuilder& TableMetadataBuilder::SetDefaultSortOrder(
465449
std::shared_ptr<SortOrder> order) {
466-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
450+
BUILDER_ASSIGN_OR_RETURN(auto order_id, AddSortOrderInternal(order));
451+
return SetDefaultSortOrder(order_id);
467452
}
468453

469454
TableMetadataBuilder& TableMetadataBuilder::SetDefaultSortOrder(int32_t order_id) {
470-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
455+
if (order_id == -1) {
456+
if (!impl_->last_added_order_id.has_value()) {
457+
impl_->errors.emplace_back(
458+
ErrorKind::kInvalidArgument,
459+
"Cannot set last added sort order: no sort order has been added");
460+
return *this;
461+
}
462+
return SetDefaultSortOrder(impl_->last_added_order_id.value());
463+
}
464+
465+
if (order_id == impl_->metadata.default_sort_order_id) {
466+
return *this;
467+
}
468+
impl_->metadata.default_sort_order_id = order_id;
469+
470+
if (impl_->last_added_order_id.has_value() &&
471+
impl_->last_added_order_id.value() == order_id) {
472+
impl_->changes.push_back(std::make_unique<table::SetDefaultSortOrder>(-1));
473+
} else {
474+
impl_->changes.push_back(std::make_unique<table::SetDefaultSortOrder>(order_id));
475+
}
476+
return *this;
471477
}
472478

473-
TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
479+
Result<int32_t> TableMetadataBuilder::AddSortOrderInternal(
474480
std::shared_ptr<SortOrder> order) {
475-
int32_t new_order_id = impl_->ReuseOrCreateNewSortOrderId(*order);
481+
int32_t new_order_id = ReuseOrCreateNewSortOrderId(*order);
476482

477483
if (impl_->sort_orders_by_id.find(new_order_id) != impl_->sort_orders_by_id.end()) {
478484
// update last_added_order_id if the order was added in this set of changes (since it
@@ -487,20 +493,20 @@ TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
487493
}
488494
impl_->last_added_order_id =
489495
is_new_order ? std::make_optional(new_order_id) : std::nullopt;
490-
return *this;
496+
return new_order_id;
491497
}
492498

493499
// Get current schema and validate the sort order against it
494-
BUILDER_ASSIGN_OR_RETURN(auto schema, impl_->metadata.Schema());
495-
BUILDER_RETURN_IF_ERROR(order->Validate(*schema));
500+
ICEBERG_ASSIGN_OR_RAISE(auto schema, impl_->metadata.Schema());
501+
ICEBERG_RETURN_UNEXPECTED(order->Validate(*schema));
496502

497503
std::shared_ptr<SortOrder> new_order;
498504
if (order->is_unsorted()) {
499505
new_order = SortOrder::Unsorted();
500506
} else {
501507
// Unlike freshSortOrder from Java impl, we don't use field name from old bound
502508
// schema to rebuild the sort order.
503-
BUILDER_ASSIGN_OR_RETURN(
509+
ICEBERG_ASSIGN_OR_RAISE(
504510
new_order,
505511
SortOrder::Make(new_order_id, std::vector<SortField>(order->fields().begin(),
506512
order->fields().end())));
@@ -511,9 +517,31 @@ TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
511517

512518
impl_->changes.push_back(std::make_unique<table::AddSortOrder>(new_order));
513519
impl_->last_added_order_id = new_order_id;
520+
return new_order_id;
521+
}
522+
523+
TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
524+
std::shared_ptr<SortOrder> order) {
525+
BUILDER_ASSIGN_OR_RETURN(auto order_id, AddSortOrderInternal(order));
514526
return *this;
515527
}
516528

529+
int32_t TableMetadataBuilder::ReuseOrCreateNewSortOrderId(const SortOrder& new_order) {
530+
if (new_order.is_unsorted()) {
531+
return SortOrder::kUnsortedOrderId;
532+
}
533+
// determine the next order id
534+
int32_t new_order_id = SortOrder::kInitialSortOrderId;
535+
for (const auto& order : impl_->metadata.sort_orders) {
536+
if (order->SameOrder(new_order)) {
537+
return order->order_id();
538+
} else if (new_order_id <= order->order_id()) {
539+
new_order_id = order->order_id() + 1;
540+
}
541+
}
542+
return new_order_id;
543+
}
544+
517545
TableMetadataBuilder& TableMetadataBuilder::AddSnapshot(
518546
std::shared_ptr<Snapshot> snapshot) {
519547
throw IcebergError(std::format("{} not implemented", __FUNCTION__));

‎src/iceberg/table_metadata.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,17 @@ class ICEBERG_EXPORT TableMetadataBuilder : public ErrorCollector {
436436
/// \brief Private constructor for building from existing metadata
437437
explicit TableMetadataBuilder(const TableMetadata* base);
438438

439+
/// \brief Internal method to add a sort order and return its ID
440+
/// \param order The sort order to add
441+
/// \return The ID of the added or reused sort order
442+
Result<int32_t> AddSortOrderInternal(std::shared_ptr<SortOrder> order);
443+
444+
/// \brief Internal method to check for existing sort order and reuse its ID or create a
445+
/// new one
446+
/// \param new_order The sort order to check
447+
/// \return The ID to use for this sort order (reused if exists, new otherwise)
448+
int32_t ReuseOrCreateNewSortOrderId(const SortOrder& new_order);
449+
439450
/// Internal state members
440451
struct Impl;
441452
std::unique_ptr<Impl> impl_;

‎src/iceberg/test/table_metadata_builder_test.cc‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,4 +259,63 @@ TEST(TableMetadataBuilderTest, ApplyAddSortOrderUpdate) {
259259
EXPECT_EQ(metadata->sort_orders[1]->order_id(), 1);
260260
}
261261

262+
// Test SetDefaultSortOrder
263+
TEST(TableMetadataBuilderTest, SetDefaultSortOrderBasic) {
264+
auto base = CreateBaseMetadata();
265+
auto schema = CreateTestSchema();
266+
267+
// 1. Set default sort order by SortOrder object
268+
auto builder = TableMetadataBuilder::BuildFrom(base.get());
269+
SortField field1(1, Transform::Identity(), SortDirection::kAscending,
270+
NullOrder::kFirst);
271+
ICEBERG_UNWRAP_OR_FAIL(auto order1_unique,
272+
SortOrder::Make(*schema, 1, std::vector<SortField>{field1}));
273+
auto order1 = std::shared_ptr<SortOrder>(std::move(order1_unique));
274+
builder->SetDefaultSortOrder(order1);
275+
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
276+
ASSERT_EQ(metadata->sort_orders.size(), 2);
277+
EXPECT_EQ(metadata->default_sort_order_id, 1);
278+
EXPECT_EQ(metadata->sort_orders[1]->order_id(), 1);
279+
280+
// 2. Set default sort order by order ID
281+
builder = TableMetadataBuilder::BuildFrom(base.get());
282+
SortField field2(1, Transform::Identity(), SortDirection::kAscending,
283+
NullOrder::kFirst);
284+
ICEBERG_UNWRAP_OR_FAIL(auto order2_unique,
285+
SortOrder::Make(*schema, 1, std::vector<SortField>{field2}));
286+
auto order2 = std::shared_ptr<SortOrder>(std::move(order2_unique));
287+
builder->AddSortOrder(order2);
288+
builder->SetDefaultSortOrder(1);
289+
ICEBERG_UNWRAP_OR_FAIL(metadata, builder->Build());
290+
EXPECT_EQ(metadata->default_sort_order_id, 1);
291+
292+
// 3. Set default sort order using -1 (last added)
293+
builder = TableMetadataBuilder::BuildFrom(base.get());
294+
SortField field3(2, Transform::Identity(), SortDirection::kDescending,
295+
NullOrder::kLast);
296+
ICEBERG_UNWRAP_OR_FAIL(auto order3_unique,
297+
SortOrder::Make(*schema, 1, std::vector<SortField>{field3}));
298+
auto order3 = std::shared_ptr<SortOrder>(std::move(order3_unique));
299+
builder->AddSortOrder(order3);
300+
builder->SetDefaultSortOrder(-1); // Use last added
301+
ICEBERG_UNWRAP_OR_FAIL(metadata, builder->Build());
302+
EXPECT_EQ(metadata->default_sort_order_id, 1);
303+
304+
// 4. Setting same order is no-op
305+
builder = TableMetadataBuilder::BuildFrom(base.get());
306+
builder->SetDefaultSortOrder(0); // Already default (unsorted)
307+
ICEBERG_UNWRAP_OR_FAIL(metadata, builder->Build());
308+
EXPECT_EQ(metadata->default_sort_order_id, 0);
309+
}
310+
311+
TEST(TableMetadataBuilderTest, SetDefaultSortOrderInvalid) {
312+
auto base = CreateBaseMetadata();
313+
314+
// 1. Try to use -1 (last added) when no order has been added
315+
auto builder = TableMetadataBuilder::BuildFrom(base.get());
316+
builder->SetDefaultSortOrder(-1);
317+
ASSERT_THAT(builder->Build(), IsError(ErrorKind::kCommitFailed));
318+
ASSERT_THAT(builder->Build(), HasErrorMessage("no sort order has been added"));
319+
}
320+
262321
} // namespace iceberg

0 commit comments

Comments
 (0)