Skip to content

Commit 3b15ddf

Browse files
HeartLinkedHeartLinked
committed
feat: implement rable_requirements
1 parent d3f7caa commit 3b15ddf

File tree

5 files changed

+286
-17
lines changed

5 files changed

+286
-17
lines changed

‎src/iceberg/table_requirements.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ class ICEBERG_EXPORT TableUpdateContext {
7373
const bool is_replace_;
7474

7575
std::vector<std::unique_ptr<TableRequirement>> requirements_;
76+
77+
// Deduplication flags to avoid adding duplicate requirements
78+
// These track whether specific requirement types have already been added
79+
bool added_last_assigned_field_id_ = false;
80+
bool added_current_schema_id_ = false;
81+
bool added_last_assigned_partition_id_ = false;
82+
bool added_default_spec_id_ = false;
83+
bool added_default_sort_order_id_ = false;
7684
};
7785

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

‎src/iceberg/table_update.cc‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,7 @@ void AssignUUID::ApplyTo(TableMetadataBuilder& builder) const {
3333
}
3434

3535
Status AssignUUID::GenerateRequirements(TableUpdateContext& context) const {
36-
// AssignUUID operation generates a requirement to assert the table's UUID
37-
// if a base metadata exists (i.e., this is an update operation)
38-
39-
const TableMetadata* base = context.base();
40-
41-
if (base != nullptr && !base->table_uuid.empty()) {
42-
// For table updates, assert that the current UUID matches what we expect
43-
context.AddRequirement(std::make_unique<AssertUUID>(base->table_uuid));
44-
}
45-
46-
// Note: For table creation (base == nullptr), no UUID requirement is needed
47-
// as the table doesn't exist yet
48-
36+
// AssignUUID does not generate additional requirements.
4937
return {};
5038
}
5139

@@ -56,7 +44,8 @@ void UpgradeFormatVersion::ApplyTo(TableMetadataBuilder& builder) const {
5644
}
5745

5846
Status UpgradeFormatVersion::GenerateRequirements(TableUpdateContext& context) const {
59-
return NotImplemented("UpgradeFormatVersion::GenerateRequirements not implemented");
47+
// AssignUUID does not generate additional requirements.
48+
return {};
6049
}
6150

6251
// AddSchema

‎src/iceberg/test/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ add_iceberg_test(table_test
8686
table_test.cc
8787
table_metadata_builder_test.cc
8888
table_requirement_test.cc
89+
table_requirements_test.cc
8990
table_update_test.cc
9091
test_common.cc)
9192

‎src/iceberg/test/table_requirements_test.cc‎

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/table_requirements.h"
21+
22+
#include <memory>
23+
#include <string>
24+
#include <vector>
25+
26+
#include <gtest/gtest.h>
27+
28+
#include "iceberg/partition_spec.h"
29+
#include "iceberg/snapshot.h"
30+
#include "iceberg/sort_order.h"
31+
#include "iceberg/table_metadata.h"
32+
#include "iceberg/table_requirement.h"
33+
#include "iceberg/table_update.h"
34+
#include "iceberg/test/matchers.h"
35+
36+
namespace iceberg {
37+
38+
namespace {
39+
40+
// Helper function to create base metadata for tests
41+
std::unique_ptr<TableMetadata> CreateBaseMetadata(
42+
const std::string& uuid = "test-uuid-1234") {
43+
auto metadata = std::make_unique<TableMetadata>();
44+
metadata->format_version = 2;
45+
metadata->table_uuid = uuid;
46+
metadata->location = "s3://bucket/test";
47+
metadata->last_sequence_number = 0;
48+
metadata->last_updated_ms = TimePointMs{std::chrono::milliseconds(1000)};
49+
metadata->last_column_id = 0;
50+
metadata->default_spec_id = PartitionSpec::kInitialSpecId;
51+
metadata->last_partition_id = 0;
52+
metadata->current_snapshot_id = Snapshot::kInvalidSnapshotId;
53+
metadata->default_sort_order_id = SortOrder::kInitialSortOrderId;
54+
metadata->next_row_id = TableMetadata::kInitialRowId;
55+
return metadata;
56+
}
57+
58+
} // namespace
59+
60+
// Test: Empty updates for CreateTable
61+
TEST(TableRequirementsTest, EmptyUpdatesForCreateTable) {
62+
std::vector<std::unique_ptr<TableUpdate>> updates;
63+
64+
auto result = TableRequirements::ForCreateTable(updates);
65+
ASSERT_THAT(result, IsOk());
66+
67+
auto& requirements = result.value();
68+
ASSERT_EQ(requirements.size(), 1);
69+
70+
// Should have only AssertDoesNotExist requirement
71+
auto* assert_does_not_exist =
72+
dynamic_cast<table::AssertDoesNotExist*>(requirements[0].get());
73+
EXPECT_NE(assert_does_not_exist, nullptr);
74+
}
75+
76+
// Test: Empty updates for UpdateTable
77+
TEST(TableRequirementsTest, EmptyUpdatesForUpdateTable) {
78+
auto metadata = CreateBaseMetadata();
79+
std::vector<std::unique_ptr<TableUpdate>> updates;
80+
81+
auto result = TableRequirements::ForUpdateTable(*metadata, updates);
82+
ASSERT_THAT(result, IsOk());
83+
84+
auto& requirements = result.value();
85+
ASSERT_EQ(requirements.size(), 1);
86+
87+
// Should have only AssertUUID requirement
88+
auto* assert_uuid = dynamic_cast<table::AssertUUID*>(requirements[0].get());
89+
ASSERT_NE(assert_uuid, nullptr);
90+
EXPECT_EQ(assert_uuid->uuid(), metadata->table_uuid);
91+
}
92+
93+
// Test: Empty updates for ReplaceTable
94+
TEST(TableRequirementsTest, EmptyUpdatesForReplaceTable) {
95+
auto metadata = CreateBaseMetadata();
96+
std::vector<std::unique_ptr<TableUpdate>> updates;
97+
98+
auto result = TableRequirements::ForReplaceTable(*metadata, updates);
99+
ASSERT_THAT(result, IsOk());
100+
101+
auto& requirements = result.value();
102+
ASSERT_EQ(requirements.size(), 1);
103+
104+
// Should have only AssertUUID requirement
105+
auto* assert_uuid = dynamic_cast<table::AssertUUID*>(requirements[0].get());
106+
ASSERT_NE(assert_uuid, nullptr);
107+
EXPECT_EQ(assert_uuid->uuid(), metadata->table_uuid);
108+
}
109+
110+
// Test: Table already exists (CreateTable requirement validation)
111+
TEST(TableRequirementsTest, TableAlreadyExists) {
112+
std::vector<std::unique_ptr<TableUpdate>> updates;
113+
114+
auto result = TableRequirements::ForCreateTable(updates);
115+
ASSERT_THAT(result, IsOk());
116+
117+
auto& requirements = result.value();
118+
ASSERT_EQ(requirements.size(), 1);
119+
120+
// Validate against existing metadata (should fail)
121+
auto metadata = CreateBaseMetadata();
122+
auto status = requirements[0]->Validate(metadata.get());
123+
EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
124+
EXPECT_THAT(status, HasErrorMessage("table already exists"));
125+
}
126+
127+
// Test: Table does not exist (CreateTable requirement validation)
128+
TEST(TableRequirementsTest, TableDoesNotExist) {
129+
std::vector<std::unique_ptr<TableUpdate>> updates;
130+
131+
auto result = TableRequirements::ForCreateTable(updates);
132+
ASSERT_THAT(result, IsOk());
133+
134+
auto& requirements = result.value();
135+
ASSERT_EQ(requirements.size(), 1);
136+
137+
// Validate against null metadata (should succeed)
138+
auto status = requirements[0]->Validate(nullptr);
139+
EXPECT_THAT(status, IsOk());
140+
}
141+
142+
// Test: AssignUUID for UpdateTable
143+
TEST(TableRequirementsTest, AssignUUID) {
144+
auto metadata = CreateBaseMetadata("original-uuid");
145+
std::vector<std::unique_ptr<TableUpdate>> updates;
146+
147+
// Add multiple AssignUUID updates
148+
updates.push_back(std::make_unique<table::AssignUUID>(metadata->table_uuid));
149+
updates.push_back(std::make_unique<table::AssignUUID>("new-uuid-1"));
150+
updates.push_back(std::make_unique<table::AssignUUID>("new-uuid-2"));
151+
152+
auto result = TableRequirements::ForUpdateTable(*metadata, updates);
153+
ASSERT_THAT(result, IsOk());
154+
155+
auto& requirements = result.value();
156+
// After deduplication: only 1 AssertUUID from ForUpdateTable
157+
ASSERT_EQ(requirements.size(), 1);
158+
159+
// Should have AssertUUID requirement with original UUID
160+
auto* assert_uuid = dynamic_cast<table::AssertUUID*>(requirements[0].get());
161+
ASSERT_NE(assert_uuid, nullptr);
162+
EXPECT_EQ(assert_uuid->uuid(), "original-uuid");
163+
164+
// Validate against base metadata (should succeed)
165+
auto status = requirements[0]->Validate(metadata.get());
166+
EXPECT_THAT(status, IsOk());
167+
}
168+
169+
// Test: AssignUUID validation failure
170+
TEST(TableRequirementsTest, AssignUUIDFailure) {
171+
auto metadata = CreateBaseMetadata("original-uuid");
172+
std::vector<std::unique_ptr<TableUpdate>> updates;
173+
updates.push_back(std::make_unique<table::AssignUUID>(metadata->table_uuid));
174+
175+
auto result = TableRequirements::ForUpdateTable(*metadata, updates);
176+
ASSERT_THAT(result, IsOk());
177+
178+
auto& requirements = result.value();
179+
// After deduplication: only 1 AssertUUID from ForUpdateTable
180+
ASSERT_EQ(requirements.size(), 1);
181+
182+
// Create updated metadata with different UUID
183+
auto updated = CreateBaseMetadata("different-uuid");
184+
185+
// Validate against updated metadata (should fail)
186+
auto status = requirements[0]->Validate(updated.get());
187+
EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
188+
EXPECT_THAT(status, HasErrorMessage("UUID does not match"));
189+
}
190+
191+
// Test: AssignUUID for ReplaceTable
192+
TEST(TableRequirementsTest, AssignUUIDForReplaceTable) {
193+
auto metadata = CreateBaseMetadata("original-uuid");
194+
std::vector<std::unique_ptr<TableUpdate>> updates;
195+
196+
// Add multiple AssignUUID updates
197+
updates.push_back(std::make_unique<table::AssignUUID>(metadata->table_uuid));
198+
updates.push_back(std::make_unique<table::AssignUUID>("new-uuid-1"));
199+
updates.push_back(std::make_unique<table::AssignUUID>("new-uuid-2"));
200+
201+
auto result = TableRequirements::ForReplaceTable(*metadata, updates);
202+
ASSERT_THAT(result, IsOk());
203+
204+
auto& requirements = result.value();
205+
// After deduplication: only 1 AssertUUID from ForReplaceTable
206+
ASSERT_EQ(requirements.size(), 1);
207+
208+
// Should have AssertUUID requirement
209+
auto* assert_uuid = dynamic_cast<table::AssertUUID*>(requirements[0].get());
210+
ASSERT_NE(assert_uuid, nullptr);
211+
EXPECT_EQ(assert_uuid->uuid(), "original-uuid");
212+
213+
// Validate against base metadata (should succeed)
214+
auto status = requirements[0]->Validate(metadata.get());
215+
EXPECT_THAT(status, IsOk());
216+
}
217+
218+
// Test: Multiple requirements validation
219+
TEST(TableRequirementsTest, ValidateAllRequirements) {
220+
auto metadata = CreateBaseMetadata("test-uuid");
221+
std::vector<std::unique_ptr<TableUpdate>> updates;
222+
updates.push_back(std::make_unique<table::AssignUUID>(metadata->table_uuid));
223+
224+
auto result = TableRequirements::ForUpdateTable(*metadata, updates);
225+
ASSERT_THAT(result, IsOk());
226+
227+
auto& requirements = result.value();
228+
229+
// All requirements should validate successfully
230+
for (const auto& req : requirements) {
231+
auto status = req->Validate(metadata.get());
232+
EXPECT_THAT(status, IsOk());
233+
}
234+
}
235+
236+
// Test: UUID comparison is case-insensitive
237+
TEST(TableRequirementsTest, AssignUUIDCaseInsensitive) {
238+
auto metadata = CreateBaseMetadata("TEST-UUID-1234");
239+
std::vector<std::unique_ptr<TableUpdate>> updates;
240+
updates.push_back(std::make_unique<table::AssignUUID>("test-uuid-1234"));
241+
242+
auto result = TableRequirements::ForUpdateTable(*metadata, updates);
243+
ASSERT_THAT(result, IsOk());
244+
245+
auto& requirements = result.value();
246+
ASSERT_EQ(requirements.size(), 1);
247+
248+
// Validate against base metadata (should succeed due to case-insensitive comparison)
249+
auto status = requirements[0]->Validate(metadata.get());
250+
EXPECT_THAT(status, IsOk());
251+
}
252+
253+
// Test: ForCreateTable with AssignUUID does not generate requirements
254+
TEST(TableRequirementsTest, CreateTableWithAssignUUID) {
255+
std::vector<std::unique_ptr<TableUpdate>> updates;
256+
updates.push_back(std::make_unique<table::AssignUUID>("new-table-uuid"));
257+
258+
auto result = TableRequirements::ForCreateTable(updates);
259+
ASSERT_THAT(result, IsOk());
260+
261+
auto& requirements = result.value();
262+
// Should have only AssertDoesNotExist, no UUID requirement for new tables
263+
ASSERT_EQ(requirements.size(), 1);
264+
265+
auto* assert_does_not_exist =
266+
dynamic_cast<table::AssertDoesNotExist*>(requirements[0].get());
267+
EXPECT_NE(assert_does_not_exist, nullptr);
268+
}
269+
270+
} // namespace iceberg

‎src/iceberg/test/table_update_test.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,15 @@ std::unique_ptr<TableMetadata> CreateBaseMetadata() {
7171
TEST(TableUpdateTest, AssignUUIDGenerateRequirements) {
7272
table::AssignUUID update("new-uuid");
7373

74-
// New table - no requirements
74+
// New table - no requirements (AssignUUID doesn't generate requirements)
7575
auto new_table_reqs = GenerateRequirements(update, nullptr);
7676
EXPECT_TRUE(new_table_reqs.empty());
7777

78-
// Existing table - should generate AssertUUID requirement
78+
// Existing table - AssignUUID doesn't generate requirements anymore
79+
// The UUID assertion is added by ForUpdateTable/ForReplaceTable methods
7980
auto base = CreateBaseMetadata();
8081
auto existing_table_reqs = GenerateRequirements(update, base.get());
81-
EXPECT_EQ(existing_table_reqs.size(), 1);
82+
EXPECT_TRUE(existing_table_reqs.empty());
8283

8384
// Existing table with empty UUID - no requirements
8485
base->table_uuid = "";

0 commit comments

Comments
 (0)