Skip to content

Commit b772da1

Browse files
HeartLinkedHeartLinked
committed
1
1 parent 60bf322 commit b772da1

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
@@ -339,22 +339,6 @@ struct TableMetadataBuilder::Impl {
339339
sort_orders_by_id.emplace(order->order_id(), order);
340340
}
341341
}
342-
343-
int32_t ReuseOrCreateNewSortOrderId(const SortOrder& new_order) {
344-
if (new_order.is_unsorted()) {
345-
return SortOrder::kUnsortedOrderId;
346-
}
347-
// determine the next order id
348-
int32_t new_order_id = SortOrder::kInitialSortOrderId;
349-
for (const auto& order : metadata.sort_orders) {
350-
if (order->SameOrder(new_order)) {
351-
return order->order_id();
352-
} else if (new_order_id <= order->order_id()) {
353-
new_order_id = order->order_id() + 1;
354-
}
355-
}
356-
return new_order_id;
357-
}
358342
};
359343

360344
TableMetadataBuilder::TableMetadataBuilder(int8_t format_version)
@@ -467,16 +451,38 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveSchemas(
467451

468452
TableMetadataBuilder& TableMetadataBuilder::SetDefaultSortOrder(
469453
std::shared_ptr<SortOrder> order) {
470-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
454+
BUILDER_ASSIGN_OR_RETURN(auto order_id, AddSortOrderInternal(order));
455+
return SetDefaultSortOrder(order_id);
471456
}
472457

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

477-
TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
483+
Result<int32_t> TableMetadataBuilder::AddSortOrderInternal(
478484
std::shared_ptr<SortOrder> order) {
479-
int32_t new_order_id = impl_->ReuseOrCreateNewSortOrderId(*order);
485+
int32_t new_order_id = ReuseOrCreateNewSortOrderId(*order);
480486

481487
if (impl_->sort_orders_by_id.find(new_order_id) != impl_->sort_orders_by_id.end()) {
482488
// update last_added_order_id if the order was added in this set of changes (since it
@@ -491,20 +497,20 @@ TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
491497
}
492498
impl_->last_added_order_id =
493499
is_new_order ? std::make_optional(new_order_id) : std::nullopt;
494-
return *this;
500+
return new_order_id;
495501
}
496502

497503
// Get current schema and validate the sort order against it
498-
BUILDER_ASSIGN_OR_RETURN(auto schema, impl_->metadata.Schema());
499-
BUILDER_RETURN_IF_ERROR(order->Validate(*schema));
504+
ICEBERG_ASSIGN_OR_RAISE(auto schema, impl_->metadata.Schema());
505+
ICEBERG_RETURN_UNEXPECTED(order->Validate(*schema));
500506

501507
std::shared_ptr<SortOrder> new_order;
502508
if (order->is_unsorted()) {
503509
new_order = SortOrder::Unsorted();
504510
} else {
505511
// Unlike freshSortOrder from Java impl, we don't use field name from old bound
506512
// schema to rebuild the sort order.
507-
BUILDER_ASSIGN_OR_RETURN(
513+
ICEBERG_ASSIGN_OR_RAISE(
508514
new_order,
509515
SortOrder::Make(new_order_id, std::vector<SortField>(order->fields().begin(),
510516
order->fields().end())));
@@ -515,9 +521,31 @@ TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
515521

516522
impl_->changes.push_back(std::make_unique<table::AddSortOrder>(new_order));
517523
impl_->last_added_order_id = new_order_id;
524+
return new_order_id;
525+
}
526+
527+
TableMetadataBuilder& TableMetadataBuilder::AddSortOrder(
528+
std::shared_ptr<SortOrder> order) {
529+
BUILDER_ASSIGN_OR_RETURN(auto order_id, AddSortOrderInternal(order));
518530
return *this;
519531
}
520532

533+
int32_t TableMetadataBuilder::ReuseOrCreateNewSortOrderId(const SortOrder& new_order) {
534+
if (new_order.is_unsorted()) {
535+
return SortOrder::kUnsortedOrderId;
536+
}
537+
// determine the next order id
538+
int32_t new_order_id = SortOrder::kInitialSortOrderId;
539+
for (const auto& order : impl_->metadata.sort_orders) {
540+
if (order->SameOrder(new_order)) {
541+
return order->order_id();
542+
} else if (new_order_id <= order->order_id()) {
543+
new_order_id = order->order_id() + 1;
544+
}
545+
}
546+
return new_order_id;
547+
}
548+
521549
TableMetadataBuilder& TableMetadataBuilder::AddSnapshot(
522550
std::shared_ptr<Snapshot> snapshot) {
523551
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
@@ -435,6 +435,17 @@ class ICEBERG_EXPORT TableMetadataBuilder {
435435
/// \brief Private constructor for building from existing metadata
436436
explicit TableMetadataBuilder(const TableMetadata* base);
437437

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