Skip to content

Commit dab2f35

Browse files
HeartLinkedHeartLinked
committed
modify
1 parent 515103c commit dab2f35

File tree

6 files changed

+137
-76
lines changed

6 files changed

+137
-76
lines changed

‎src/iceberg/table_metadata.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ namespace iceberg {
4747

4848
#define BUILDER_RETURN_IF_ERROR(result) \
4949
if (auto&& result_name = result; !result_name) [[unlikely]] { \
50-
impl_->errors.emplace_back(std::move(result_name.error())); \
50+
errors_.emplace_back(std::move(result_name.error())); \
5151
return *this; \
5252
}
5353

@@ -454,7 +454,7 @@ TableMetadataBuilder& TableMetadataBuilder::SetDefaultSortOrder(
454454
TableMetadataBuilder& TableMetadataBuilder::SetDefaultSortOrder(int32_t order_id) {
455455
if (order_id == -1) {
456456
if (!impl_->last_added_order_id.has_value()) {
457-
impl_->errors.emplace_back(
457+
errors_.emplace_back(
458458
ErrorKind::kInvalidArgument,
459459
"Cannot set last added sort order: no sort order has been added");
460460
return *this;

‎src/iceberg/table_requirements.cc‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,56 @@ Result<std::vector<std::unique_ptr<TableRequirement>>> TableUpdateContext::Build
3535
return std::move(requirements_);
3636
}
3737

38+
void TableUpdateContext::RequireLastAssignedFieldIdUnchanged() {
39+
if (!asserted_last_assigned_field_id_) {
40+
if (base_ != nullptr) {
41+
AddRequirement(
42+
std::make_unique<table::AssertLastAssignedFieldId>(base_->last_column_id));
43+
}
44+
asserted_last_assigned_field_id_ = true;
45+
}
46+
}
47+
48+
void TableUpdateContext::RequireCurrentSchemaIdUnchanged() {
49+
if (!asserted_current_schema_id_) {
50+
if (base_ != nullptr && !is_replace_) {
51+
AddRequirement(std::make_unique<table::AssertCurrentSchemaID>(
52+
base_->current_schema_id.value()));
53+
}
54+
asserted_current_schema_id_ = true;
55+
}
56+
}
57+
58+
void TableUpdateContext::RequireLastAssignedPartitionIdUnchanged() {
59+
if (!asserted_last_assigned_partition_id_) {
60+
if (base_ != nullptr) {
61+
AddRequirement(std::make_unique<table::AssertLastAssignedPartitionId>(
62+
base_->last_partition_id));
63+
}
64+
asserted_last_assigned_partition_id_ = true;
65+
}
66+
}
67+
68+
void TableUpdateContext::RequireDefaultSpecIdUnchanged() {
69+
if (!asserted_default_spec_id_) {
70+
if (base_ != nullptr && !is_replace_) {
71+
AddRequirement(
72+
std::make_unique<table::AssertDefaultSpecID>(base_->default_spec_id));
73+
}
74+
asserted_default_spec_id_ = true;
75+
}
76+
}
77+
78+
void TableUpdateContext::RequireDefaultSortOrderIdUnchanged() {
79+
if (!asserted_default_sort_order_id_) {
80+
if (base_ != nullptr && !is_replace_) {
81+
AddRequirement(std::make_unique<table::AssertDefaultSortOrderID>(
82+
base_->default_sort_order_id));
83+
}
84+
asserted_default_sort_order_id_ = true;
85+
}
86+
}
87+
3888
Result<std::vector<std::unique_ptr<TableRequirement>>> TableRequirements::ForCreateTable(
3989
const std::vector<std::unique_ptr<TableUpdate>>& table_updates) {
4090
TableUpdateContext context(nullptr, false);

‎src/iceberg/table_requirements.h‎

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,27 +68,17 @@ class ICEBERG_EXPORT TableUpdateContext {
6868
/// \brief Build and return the list of requirements
6969
Result<std::vector<std::unique_ptr<TableRequirement>>> Build();
7070

71-
// Getters for deduplication flags
72-
bool added_last_assigned_field_id() const { return added_last_assigned_field_id_; }
73-
bool added_current_schema_id() const { return added_current_schema_id_; }
74-
bool added_last_assigned_partition_id() const {
75-
return added_last_assigned_partition_id_;
76-
}
77-
bool added_default_spec_id() const { return added_default_spec_id_; }
78-
bool added_default_sort_order_id() const { return added_default_sort_order_id_; }
79-
80-
// Setters for deduplication flags
81-
void set_added_last_assigned_field_id(bool value) {
82-
added_last_assigned_field_id_ = value;
83-
}
84-
void set_added_current_schema_id(bool value) { added_current_schema_id_ = value; }
85-
void set_added_last_assigned_partition_id(bool value) {
86-
added_last_assigned_partition_id_ = value;
87-
}
88-
void set_added_default_spec_id(bool value) { added_default_spec_id_ = value; }
89-
void set_added_default_sort_order_id(bool value) {
90-
added_default_sort_order_id_ = value;
91-
}
71+
// Helper methods to add requirements with deduplication
72+
/// \brief Require that the last assigned field ID remains unchanged
73+
void RequireLastAssignedFieldIdUnchanged();
74+
/// \brief Require that the current schema ID remains unchanged
75+
void RequireCurrentSchemaIdUnchanged();
76+
/// \brief Require that the last assigned partition ID remains unchanged
77+
void RequireLastAssignedPartitionIdUnchanged();
78+
/// \brief Require that the default spec ID remains unchanged
79+
void RequireDefaultSpecIdUnchanged();
80+
/// \brief Require that the default sort order ID remains unchanged
81+
void RequireDefaultSortOrderIdUnchanged();
9282

9383
private:
9484
const TableMetadata* base_;
@@ -97,11 +87,11 @@ class ICEBERG_EXPORT TableUpdateContext {
9787
std::vector<std::unique_ptr<TableRequirement>> requirements_;
9888

9989
// flags to avoid adding duplicate requirements
100-
bool added_last_assigned_field_id_ = false;
101-
bool added_current_schema_id_ = false;
102-
bool added_last_assigned_partition_id_ = false;
103-
bool added_default_spec_id_ = false;
104-
bool added_default_sort_order_id_ = false;
90+
bool asserted_last_assigned_field_id_ = false;
91+
bool asserted_current_schema_id_ = false;
92+
bool asserted_last_assigned_partition_id_ = false;
93+
bool asserted_default_spec_id_ = false;
94+
bool asserted_default_sort_order_id_ = false;
10595
};
10696

10797
/// \brief Factory class for generating table requirements

‎src/iceberg/table_update.cc‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,7 @@ void SetDefaultSortOrder::ApplyTo(TableMetadataBuilder& builder) const {
127127
}
128128

129129
Status SetDefaultSortOrder::GenerateRequirements(TableUpdateContext& context) const {
130-
const TableMetadata* base = context.base();
131-
if (base != nullptr && !context.is_replace()) {
132-
context.AddRequirement(
133-
// For table updates, assert that the current default sort order ID matches what
134-
// we expect
135-
std::make_unique<AssertDefaultSortOrderID>(base->default_sort_order_id));
136-
}
130+
context.RequireDefaultSortOrderIdUnchanged();
137131
return {};
138132
}
139133

‎src/iceberg/test/table_metadata_builder_test.cc‎

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ TEST(TableMetadataBuilderTest, BuildFromExisting) {
9898
EXPECT_EQ(metadata->location, "s3://bucket/test");
9999
}
100100

101-
// Test AssignUUID method
101+
// Test AssignUUID
102102
TEST(TableMetadataBuilderTest, AssignUUID) {
103103
// Assign UUID for new table
104104
auto builder = TableMetadataBuilder::BuildFromEmpty(2);
@@ -226,39 +226,6 @@ TEST(TableMetadataBuilderTest, AddSortOrderInvalid) {
226226
ASSERT_THAT(builder->Build(), HasErrorMessage("Schema with ID"));
227227
}
228228

229-
// Test applying AssignUUID update to builder
230-
TEST(TableMetadataBuilderTest, ApplyAssignUUIDUpdate) {
231-
auto base = CreateBaseMetadata();
232-
auto builder = TableMetadataBuilder::BuildFrom(base.get());
233-
234-
// Apply AssignUUID update
235-
table::AssignUUID uuid_update("apply-uuid");
236-
uuid_update.ApplyTo(*builder);
237-
238-
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
239-
EXPECT_EQ(metadata->table_uuid, "apply-uuid");
240-
}
241-
242-
// Test applying AddSortOrder update to builder
243-
TEST(TableMetadataBuilderTest, ApplyAddSortOrderUpdate) {
244-
auto base = CreateBaseMetadata();
245-
auto builder = TableMetadataBuilder::BuildFrom(base.get());
246-
247-
// Apply AddSortOrder update
248-
auto schema = CreateTestSchema();
249-
SortField sort_field(1, Transform::Identity(), SortDirection::kAscending,
250-
NullOrder::kFirst);
251-
ICEBERG_UNWRAP_OR_FAIL(auto sort_order_unique,
252-
SortOrder::Make(*schema, 1, std::vector<SortField>{sort_field}));
253-
auto sort_order = std::shared_ptr<SortOrder>(std::move(sort_order_unique));
254-
table::AddSortOrder sort_order_update(sort_order);
255-
sort_order_update.ApplyTo(*builder);
256-
257-
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
258-
ASSERT_EQ(metadata->sort_orders.size(), 2);
259-
EXPECT_EQ(metadata->sort_orders[1]->order_id(), 1);
260-
}
261-
262229
// Test SetDefaultSortOrder
263230
TEST(TableMetadataBuilderTest, SetDefaultSortOrderBasic) {
264231
auto base = CreateBaseMetadata();

‎src/iceberg/test/table_update_test.cc‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,23 +82,50 @@ std::unique_ptr<TableMetadata> CreateBaseMetadata() {
8282

8383
} // namespace
8484

85+
// Test AssignUUID
86+
TEST(TableMetadataBuilderTest, AssignUUIDApplyUpdate) {
87+
auto base = CreateBaseMetadata();
88+
auto builder = TableMetadataBuilder::BuildFrom(base.get());
89+
90+
// Apply AssignUUID update
91+
table::AssignUUID uuid_update("apply-uuid");
92+
uuid_update.ApplyTo(*builder);
93+
94+
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
95+
EXPECT_EQ(metadata->table_uuid, "apply-uuid");
96+
}
97+
8598
TEST(TableUpdateTest, AssignUUIDGenerateRequirements) {
8699
table::AssignUUID update("new-uuid");
87100

88101
// New table - no requirements (AssignUUID doesn't generate requirements)
89102
auto new_table_reqs = GenerateRequirements(update, nullptr);
90103
EXPECT_TRUE(new_table_reqs.empty());
91104

92-
// Existing table - AssignUUID doesn't generate requirements anymore
93-
// The UUID assertion is added by ForUpdateTable/ForReplaceTable methods
105+
// Existing table - no requirements
94106
auto base = CreateBaseMetadata();
95107
auto existing_table_reqs = GenerateRequirements(update, base.get());
96108
EXPECT_TRUE(existing_table_reqs.empty());
109+
}
110+
111+
// Test AddSortOrder
112+
TEST(TableMetadataBuilderTest, ApplyAddSortOrderUpdate) {
113+
auto base = CreateBaseMetadata();
114+
auto builder = TableMetadataBuilder::BuildFrom(base.get());
97115

98-
// Existing table with empty UUID - no requirements
99-
base->table_uuid = "";
100-
auto empty_uuid_reqs = GenerateRequirements(update, base.get());
101-
EXPECT_TRUE(empty_uuid_reqs.empty());
116+
// Apply AddSortOrder update
117+
auto schema = CreateTestSchema();
118+
SortField sort_field(1, Transform::Identity(), SortDirection::kAscending,
119+
NullOrder::kFirst);
120+
ICEBERG_UNWRAP_OR_FAIL(auto sort_order_unique,
121+
SortOrder::Make(*schema, 1, std::vector<SortField>{sort_field}));
122+
auto sort_order = std::shared_ptr<SortOrder>(std::move(sort_order_unique));
123+
table::AddSortOrder sort_order_update(sort_order);
124+
sort_order_update.ApplyTo(*builder);
125+
126+
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
127+
ASSERT_EQ(metadata->sort_orders.size(), 2);
128+
EXPECT_EQ(metadata->sort_orders[1]->order_id(), 1);
102129
}
103130

104131
TEST(TableUpdateTest, AddSortOrderGenerateRequirements) {
@@ -113,7 +140,40 @@ TEST(TableUpdateTest, AddSortOrderGenerateRequirements) {
113140
auto new_table_reqs = GenerateRequirements(update, nullptr);
114141
EXPECT_TRUE(new_table_reqs.empty());
115142

116-
// Existing table - no requirements (AddSortOrder doesn't generate requirements)
143+
// Existing table - no requirements
144+
auto base = CreateBaseMetadata();
145+
auto existing_table_reqs = GenerateRequirements(update, base.get());
146+
EXPECT_TRUE(existing_table_reqs.empty());
147+
}
148+
149+
// Test SetDefaultSortOrder
150+
TEST(TableMetadataBuilderTest, ApplySetDefaultSortOrderUpdate) {
151+
auto base = CreateBaseMetadata();
152+
auto builder = TableMetadataBuilder::BuildFrom(base.get());
153+
154+
auto schema = CreateTestSchema();
155+
SortField sort_field(1, Transform::Identity(), SortDirection::kAscending,
156+
NullOrder::kFirst);
157+
ICEBERG_UNWRAP_OR_FAIL(auto sort_order_unique,
158+
SortOrder::Make(*schema, 1, std::vector<SortField>{sort_field}));
159+
auto sort_order = std::shared_ptr<SortOrder>(std::move(sort_order_unique));
160+
builder->AddSortOrder(sort_order);
161+
162+
table::SetDefaultSortOrder set_default_update(1);
163+
set_default_update.ApplyTo(*builder);
164+
165+
ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build());
166+
EXPECT_EQ(metadata->default_sort_order_id, 1);
167+
}
168+
169+
TEST(TableUpdateTest, SetDefaultSortOrderGenerateRequirements) {
170+
table::SetDefaultSortOrder update(1);
171+
172+
// New table - no requirements
173+
auto new_table_reqs = GenerateRequirements(update, nullptr);
174+
EXPECT_TRUE(new_table_reqs.empty());
175+
176+
// Existing table - no requirements
117177
auto base = CreateBaseMetadata();
118178
auto existing_table_reqs = GenerateRequirements(update, base.get());
119179
EXPECT_TRUE(existing_table_reqs.empty());

0 commit comments

Comments
 (0)