Skip to content

Commit 10cc2e0

Browse files
authored
feat: implement TableMetadataBuilder with AssignUUID (#268)
This commit implements the core TableMetadataBuilder pattern following the established design from the previous session. Pattern established: 1. Builder validates input and collects errors (no exceptions) 2. Changes are tracked for serialization 3. Requirements generated for optimistic concurrency 4. Build() validates metadata consistency before returning
1 parent e393cb4 commit 10cc2e0

File tree

12 files changed

+409
-28
lines changed

12 files changed

+409
-28
lines changed

src/iceberg/partition_spec.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class ICEBERG_EXPORT PartitionSpec : public util::Formattable {
4747
/// \brief The start ID for partition field. It is only used to generate
4848
/// partition field id for v1 metadata where it is tracked.
4949
static constexpr int32_t kLegacyPartitionDataIdStart = 1000;
50+
static constexpr int32_t kInvalidPartitionFieldId = -1;
5051

5152
/// \brief Create a new partition spec.
5253
///

src/iceberg/schema.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ namespace iceberg {
4646
class ICEBERG_EXPORT Schema : public StructType {
4747
public:
4848
static constexpr int32_t kInitialSchemaId = 0;
49+
static constexpr int32_t kInvalidColumnId = -1;
4950

5051
explicit Schema(std::vector<SchemaField> fields,
5152
std::optional<int32_t> schema_id = std::nullopt);

src/iceberg/table_metadata.cc

Lines changed: 90 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "iceberg/table_metadata.h"
2121

2222
#include <algorithm>
23+
#include <chrono>
2324
#include <format>
2425
#include <string>
2526

@@ -36,9 +37,14 @@
3637
#include "iceberg/table_update.h"
3738
#include "iceberg/util/gzip_internal.h"
3839
#include "iceberg/util/macros.h"
40+
#include "iceberg/util/uuid.h"
3941

4042
namespace iceberg {
4143

44+
namespace {
45+
const TimePointMs kInvalidLastUpdatedMs = TimePointMs::min();
46+
}
47+
4248
std::string ToString(const SnapshotLogEntry& entry) {
4349
return std::format("SnapshotLogEntry[timestampMillis={},snapshotId={}]",
4450
entry.timestamp_ms, entry.snapshot_id);
@@ -201,13 +207,46 @@ Status TableMetadataUtil::Write(FileIO& io, const std::string& location,
201207

202208
// TableMetadataBuilder implementation
203209

204-
struct TableMetadataBuilder::Impl {};
210+
struct TableMetadataBuilder::Impl {
211+
// Base metadata (nullptr for new tables)
212+
const TableMetadata* base;
213+
214+
// Working metadata copy
215+
TableMetadata metadata;
216+
217+
// Change tracking
218+
std::vector<std::unique_ptr<TableUpdate>> changes;
219+
220+
// Error collection (since methods return *this and cannot throw)
221+
std::vector<Error> errors;
222+
223+
// Metadata location tracking
224+
std::optional<std::string> metadata_location;
225+
std::optional<std::string> previous_metadata_location;
226+
227+
// Constructor for new table
228+
explicit Impl(int8_t format_version) : base(nullptr), metadata{} {
229+
metadata.format_version = format_version;
230+
metadata.last_sequence_number = TableMetadata::kInitialSequenceNumber;
231+
metadata.last_updated_ms = kInvalidLastUpdatedMs;
232+
metadata.last_column_id = Schema::kInvalidColumnId;
233+
metadata.default_spec_id = PartitionSpec::kInitialSpecId;
234+
metadata.last_partition_id = PartitionSpec::kInvalidPartitionFieldId;
235+
metadata.current_snapshot_id = Snapshot::kInvalidSnapshotId;
236+
metadata.default_sort_order_id = SortOrder::kInitialSortOrderId;
237+
metadata.next_row_id = TableMetadata::kInitialRowId;
238+
}
239+
240+
// Constructor from existing metadata
241+
explicit Impl(const TableMetadata* base_metadata)
242+
: base(base_metadata), metadata(*base_metadata) {}
243+
};
205244

206245
TableMetadataBuilder::TableMetadataBuilder(int8_t format_version)
207-
: impl_(std::make_unique<Impl>()) {}
246+
: impl_(std::make_unique<Impl>(format_version)) {}
208247

209248
TableMetadataBuilder::TableMetadataBuilder(const TableMetadata* base)
210-
: impl_(std::make_unique<Impl>()) {}
249+
: impl_(std::make_unique<Impl>(base)) {}
211250

212251
TableMetadataBuilder::~TableMetadataBuilder() = default;
213252

@@ -238,12 +277,35 @@ TableMetadataBuilder& TableMetadataBuilder::SetPreviousMetadataLocation(
238277
}
239278

240279
TableMetadataBuilder& TableMetadataBuilder::AssignUUID() {
241-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
280+
if (impl_->metadata.table_uuid.empty()) {
281+
// Generate a random UUID
282+
return AssignUUID(Uuid::GenerateV4().ToString());
283+
}
284+
285+
return *this;
242286
}
243287

244288
TableMetadataBuilder& TableMetadataBuilder::AssignUUID(std::string_view uuid) {
245-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
246-
;
289+
std::string uuid_str(uuid);
290+
291+
// Validation: UUID cannot be empty
292+
if (uuid_str.empty()) {
293+
impl_->errors.emplace_back(ErrorKind::kInvalidArgument, "Cannot assign empty UUID");
294+
return *this;
295+
}
296+
297+
// Check if UUID is already set to the same value (no-op)
298+
if (StringUtils::EqualsIgnoreCase(impl_->metadata.table_uuid, uuid_str)) {
299+
return *this;
300+
}
301+
302+
// Update the metadata
303+
impl_->metadata.table_uuid = uuid_str;
304+
305+
// Record the change
306+
impl_->changes.push_back(std::make_unique<table::AssignUUID>(std::move(uuid_str)));
307+
308+
return *this;
247309
}
248310

249311
TableMetadataBuilder& TableMetadataBuilder::UpgradeFormatVersion(
@@ -377,12 +439,29 @@ TableMetadataBuilder& TableMetadataBuilder::RemoveEncryptionKey(std::string_view
377439
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
378440
}
379441

380-
TableMetadataBuilder& TableMetadataBuilder::DiscardChanges() {
381-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
382-
}
383-
384442
Result<std::unique_ptr<TableMetadata>> TableMetadataBuilder::Build() {
385-
return NotImplemented("TableMetadataBuilder::Build not implemented");
443+
// 1. Check for accumulated errors
444+
if (!impl_->errors.empty()) {
445+
std::string error_msg = "Failed to build TableMetadata due to validation errors:\n";
446+
for (const auto& [kind, message] : impl_->errors) {
447+
error_msg += " - " + message + "\n";
448+
}
449+
return CommitFailed("{}", error_msg);
450+
}
451+
452+
// 2. Validate metadata consistency through TableMetadata#Validate
453+
454+
// 3. Update last_updated_ms if there are changes
455+
if (impl_->metadata.last_updated_ms == kInvalidLastUpdatedMs) {
456+
impl_->metadata.last_updated_ms =
457+
TimePointMs{std::chrono::duration_cast<std::chrono::milliseconds>(
458+
std::chrono::system_clock::now().time_since_epoch())};
459+
}
460+
461+
// 4. Create and return the TableMetadata
462+
auto result = std::make_unique<TableMetadata>(std::move(impl_->metadata));
463+
464+
return result;
386465
}
387466

388467
} // namespace iceberg

src/iceberg/table_metadata.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -377,13 +377,6 @@ class ICEBERG_EXPORT TableMetadataBuilder {
377377
/// \return Reference to this builder for method chaining
378378
TableMetadataBuilder& RemoveEncryptionKey(std::string_view key_id);
379379

380-
/// \brief Discard all accumulated changes
381-
///
382-
/// This is useful when you want to reset the builder state without
383-
/// creating a new builder instance.
384-
/// \return Reference to this builder for method chaining
385-
TableMetadataBuilder& DiscardChanges();
386-
387380
/// \brief Build the TableMetadata object
388381
///
389382
/// \return A Result containing the constructed TableMetadata or an error

src/iceberg/table_requirement.cc

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,28 @@
2020
#include "iceberg/table_requirement.h"
2121

2222
#include "iceberg/table_metadata.h"
23+
#include "iceberg/util/string_util.h"
2324

2425
namespace iceberg::table {
2526

2627
Status AssertDoesNotExist::Validate(const TableMetadata* base) const {
27-
return NotImplemented("AssertTableDoesNotExist::Validate not implemented");
28+
return NotImplemented("AssertDoesNotExist::Validate not implemented");
2829
}
2930

3031
Status AssertUUID::Validate(const TableMetadata* base) const {
31-
return NotImplemented("AssertTableUUID::Validate not implemented");
32+
// Validate that the table UUID matches the expected value
33+
34+
if (base == nullptr) {
35+
return CommitFailed("Requirement failed: current table metadata is missing");
36+
}
37+
38+
if (!StringUtils::EqualsIgnoreCase(base->table_uuid, uuid_)) {
39+
return CommitFailed(
40+
"Requirement failed: table UUID does not match (expected='{}', actual='{}')",
41+
uuid_, base->table_uuid);
42+
}
43+
44+
return {};
3245
}
3346

3447
Status AssertRefSnapshotID::Validate(const TableMetadata* base) const {

src/iceberg/table_requirements.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
namespace iceberg {
2727

2828
void TableUpdateContext::AddRequirement(std::unique_ptr<TableRequirement> requirement) {
29-
throw IcebergError("TableUpdateContext::AddRequirement not implemented");
29+
requirements_.emplace_back(std::move(requirement));
3030
}
3131

3232
Result<std::vector<std::unique_ptr<TableRequirement>>> TableUpdateContext::Build() {
33-
return NotImplemented("TableUpdateContext::Build not implemented");
33+
return std::move(requirements_);
3434
}
3535

3636
Result<std::vector<std::unique_ptr<TableRequirement>>> TableRequirements::ForCreateTable(

src/iceberg/table_update.cc

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,32 @@
2121

2222
#include "iceberg/exception.h"
2323
#include "iceberg/table_metadata.h"
24+
#include "iceberg/table_requirement.h"
2425
#include "iceberg/table_requirements.h"
2526

2627
namespace iceberg::table {
2728

2829
// AssignUUID
2930

3031
void AssignUUID::ApplyTo(TableMetadataBuilder& builder) const {
31-
throw IcebergError(std::format("{} not implemented", __FUNCTION__));
32+
builder.AssignUUID(uuid_);
3233
}
3334

3435
Status AssignUUID::GenerateRequirements(TableUpdateContext& context) const {
35-
return NotImplemented("AssignTableUUID::GenerateRequirements not implemented");
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+
49+
return {};
3650
}
3751

3852
// UpgradeFormatVersion
@@ -42,8 +56,7 @@ void UpgradeFormatVersion::ApplyTo(TableMetadataBuilder& builder) const {
4256
}
4357

4458
Status UpgradeFormatVersion::GenerateRequirements(TableUpdateContext& context) const {
45-
return NotImplemented(
46-
"UpgradeTableFormatVersion::GenerateRequirements not implemented");
59+
return NotImplemented("UpgradeFormatVersion::GenerateRequirements not implemented");
4760
}
4861

4962
// AddSchema

src/iceberg/test/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ add_iceberg_test(table_test
8282
test_common.cc
8383
json_internal_test.cc
8484
table_test.cc
85-
schema_json_test.cc)
85+
schema_json_test.cc
86+
table_metadata_builder_test.cc)
8687

8788
add_iceberg_test(expression_test
8889
SOURCES

src/iceberg/test/matchers.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <gtest/gtest.h>
2424

2525
#include "iceberg/result.h"
26+
#include "iceberg/util/macros.h"
2627

2728
/*
2829
* \brief Define custom matchers for expected<T, Error> values
@@ -210,4 +211,17 @@ auto ErrorIs(MatcherT&& matcher) {
210211
ResultMatcher<std::decay_t<MatcherT>>(false, std::forward<MatcherT>(matcher)));
211212
}
212213

214+
// Evaluate `rexpr` which should return a Result<T, Error>.
215+
// On success: assign the contained value to `lhs`.
216+
// On failure: fail the test with the error message.
217+
#define ICEBERG_UNWRAP_OR_FAIL_IMPL(result_name, lhs, rexpr) \
218+
auto&& result_name = (rexpr); \
219+
ASSERT_TRUE(result_name.has_value()) \
220+
<< "Operation failed: " << result_name.error().message; \
221+
lhs = std::move(result_name.value());
222+
223+
#define ICEBERG_UNWRAP_OR_FAIL(lhs, rexpr) \
224+
ICEBERG_UNWRAP_OR_FAIL_IMPL(ICEBERG_ASSIGN_OR_RAISE_NAME(result_, __COUNTER__), lhs, \
225+
rexpr)
226+
213227
} // namespace iceberg

src/iceberg/test/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ iceberg_tests = {
4747
'sources': files(
4848
'json_internal_test.cc',
4949
'schema_json_test.cc',
50+
'table_metadata_builder_test.cc',
5051
'table_test.cc',
5152
'test_common.cc',
5253
),

0 commit comments

Comments
 (0)