This is an automated email from the ASF dual-hosted git repository.
gangwu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 8388f32 feat: implement validation for table update requirements
(#294)
8388f32 is described below
commit 8388f3286b65d13dc8894162b5adfdc0aa681320
Author: Xinli Shang <[email protected]>
AuthorDate: Tue Nov 11 18:19:15 2025 -0800
feat: implement validation for table update requirements (#294)
Implement Validate() methods for remaining table requirement classes:
- AssertDoesNotExist: validates table doesn't exist
- AssertRefSnapshotID: validates snapshot references (branches/tags)
- AssertLastAssignedFieldId: validates last assigned field ID
- AssertLastAssignedPartitionId: validates last assigned partition ID
- AssertDefaultSpecID: validates default partition spec ID
- AssertDefaultSortOrderID: validates default sort order ID
All implementations follow the pattern established in
AssertCurrentSchemaID and provide descriptive error messages for
validation failures.
---
src/iceberg/table_requirement.cc | 90 +++++++++++---
src/iceberg/test/table_metadata_builder_test.cc | 149 ++++++++++++++++++++++++
2 files changed, 224 insertions(+), 15 deletions(-)
diff --git a/src/iceberg/table_requirement.cc b/src/iceberg/table_requirement.cc
index 0b8e795..3c0a01e 100644
--- a/src/iceberg/table_requirement.cc
+++ b/src/iceberg/table_requirement.cc
@@ -19,43 +19,73 @@
#include "iceberg/table_requirement.h"
+#include "iceberg/snapshot.h"
#include "iceberg/table_metadata.h"
+#include "iceberg/util/macros.h"
#include "iceberg/util/string_util.h"
namespace iceberg::table {
Status AssertDoesNotExist::Validate(const TableMetadata* base) const {
- return NotImplemented("AssertDoesNotExist::Validate not implemented");
+ if (base != nullptr) {
+ return CommitFailed("Requirement failed: table already exists");
+ }
+
+ return {};
}
Status AssertUUID::Validate(const TableMetadata* base) const {
- // Validate that the table UUID matches the expected value
-
if (base == nullptr) {
return CommitFailed("Requirement failed: current table metadata is
missing");
}
if (!StringUtils::EqualsIgnoreCase(base->table_uuid, uuid_)) {
return CommitFailed(
- "Requirement failed: table UUID does not match (expected='{}',
actual='{}')",
- uuid_, base->table_uuid);
+ "Requirement failed: table UUID does not match, expected {} != {}",
uuid_,
+ base->table_uuid);
}
return {};
}
Status AssertRefSnapshotID::Validate(const TableMetadata* base) const {
- return NotImplemented("AssertTableRefSnapshotID::Validate not implemented");
+ if (base == nullptr) {
+ return CommitFailed("Requirement failed: current table metadata is
missing");
+ }
+
+ if (auto ref = base->refs.find(ref_name_); ref != base->refs.cend()) {
+ const auto& snapshot_ref = ref->second;
+ ICEBERG_DCHECK(snapshot_ref != nullptr, "Snapshot reference is null");
+ std::string_view type =
+ snapshot_ref->type() == SnapshotRefType::kBranch ? "branch" : "tag";
+ if (!snapshot_id_.has_value()) {
+ // A null snapshot ID means the ref should not exist already
+ return CommitFailed("Requirement failed: {} '{}' was created
concurrently", type,
+ ref_name_);
+ } else if (snapshot_id_.value() != snapshot_ref->snapshot_id) {
+ return CommitFailed("Requirement failed: {} '{}' has changed: expected
id {} != {}",
+ type, ref_name_, snapshot_id_.value(),
+ snapshot_ref->snapshot_id);
+ }
+ } else if (snapshot_id_.has_value()) {
+ return CommitFailed("Requirement failed: branch or tag '{}' is missing,
expected {}",
+ ref_name_, snapshot_id_.value());
+ }
+
+ return {};
}
Status AssertLastAssignedFieldId::Validate(const TableMetadata* base) const {
- return NotImplemented(
- "AssertCurrentTableLastAssignedFieldId::Validate not implemented");
+ if (base && base->last_column_id != last_assigned_field_id_) {
+ return CommitFailed(
+ "Requirement failed: last assigned field ID does not match, expected
{} != {}",
+ last_assigned_field_id_, base->last_column_id);
+ }
+
+ return {};
}
Status AssertCurrentSchemaID::Validate(const TableMetadata* base) const {
- // Validate that the current schema ID matches the one used when the
metadata was read
-
if (base == nullptr) {
return CommitFailed("Requirement failed: current table metadata is
missing");
}
@@ -67,7 +97,7 @@ Status AssertCurrentSchemaID::Validate(const TableMetadata*
base) const {
if (base->current_schema_id.value() != schema_id_) {
return CommitFailed(
- "Requirement failed: current schema ID does not match (expected={},
actual={})",
+ "Requirement failed: current schema ID does not match, expected {} !=
{}",
schema_id_, base->current_schema_id.value());
}
@@ -75,16 +105,46 @@ Status AssertCurrentSchemaID::Validate(const
TableMetadata* base) const {
}
Status AssertLastAssignedPartitionId::Validate(const TableMetadata* base)
const {
- return NotImplemented(
- "AssertCurrentTableLastAssignedPartitionId::Validate not implemented");
+ if (base == nullptr) {
+ return CommitFailed("Requirement failed: current table metadata is
missing");
+ }
+
+ if (base && base->last_partition_id != last_assigned_partition_id_) {
+ return CommitFailed(
+ "Requirement failed: last assigned partition ID does not match,
expected {} != "
+ "{}",
+ last_assigned_partition_id_, base->last_partition_id);
+ }
+
+ return {};
}
Status AssertDefaultSpecID::Validate(const TableMetadata* base) const {
- return NotImplemented("AssertDefaultTableSpecID::Validate not implemented");
+ if (base == nullptr) {
+ return CommitFailed("Requirement failed: current table metadata is
missing");
+ }
+
+ if (base->default_spec_id != spec_id_) {
+ return CommitFailed(
+ "Requirement failed: default partition spec changed, expected id {} !=
{}",
+ spec_id_, base->default_spec_id);
+ }
+
+ return {};
}
Status AssertDefaultSortOrderID::Validate(const TableMetadata* base) const {
- return NotImplemented("AssertDefaultTableSortOrderID::Validate not
implemented");
+ if (base == nullptr) {
+ return CommitFailed("Requirement failed: current table metadata is
missing");
+ }
+
+ if (base->default_sort_order_id != sort_order_id_) {
+ return CommitFailed(
+ "Requirement failed: default sort order changed: expected id {} != {}",
+ sort_order_id_, base->default_sort_order_id);
+ }
+
+ return {};
}
} // namespace iceberg::table
diff --git a/src/iceberg/test/table_metadata_builder_test.cc
b/src/iceberg/test/table_metadata_builder_test.cc
index 6b2314f..9b98048 100644
--- a/src/iceberg/test/table_metadata_builder_test.cc
+++ b/src/iceberg/test/table_metadata_builder_test.cc
@@ -264,6 +264,155 @@ TEST_F(TableMetadataBuilderTest,
TableRequirementAssertCurrentSchemaIDNotSet) {
EXPECT_THAT(status, HasErrorMessage("schema ID is not set"));
}
+TEST_F(TableMetadataBuilderTest, TableRequirementAssertDoesNotExistSuccess) {
+ table::AssertDoesNotExist requirement;
+
+ ASSERT_THAT(requirement.Validate(nullptr), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertDoesNotExistTableExists) {
+ table::AssertDoesNotExist requirement;
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("table already exists"));
+}
+
+TEST_F(TableMetadataBuilderTest, TableRequirementAssertRefSnapshotIDSuccess) {
+ auto ref = std::make_shared<SnapshotRef>();
+ ref->snapshot_id = 100;
+ ref->retention = SnapshotRef::Branch{};
+ base_metadata_->refs["main"] = ref;
+
+ table::AssertRefSnapshotID requirement("main", 100);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest, TableRequirementAssertRefSnapshotIDMismatch) {
+ auto ref = std::make_shared<SnapshotRef>();
+ ref->snapshot_id = 100;
+ ref->retention = SnapshotRef::Branch{};
+ base_metadata_->refs["main"] = ref;
+
+ table::AssertRefSnapshotID requirement("main", 200);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("has changed"));
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertRefSnapshotIDRefMissing) {
+ table::AssertRefSnapshotID requirement("missing-ref", 100);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("is missing"));
+}
+
+// Removed TableRequirementAssertRefSnapshotIDNullBase test
+// Java implementation doesn't check for null base, so passing nullptr would
cause
+// undefined behavior This matches Java's assumption that base is never null
when Validate
+// is called
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertRefSnapshotIDNulloptSuccess) {
+ // Ref should not exist, and it doesn't
+ table::AssertRefSnapshotID requirement("nonexistent", std::nullopt);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertRefSnapshotIDNulloptButExists) {
+ auto ref = std::make_shared<SnapshotRef>();
+ ref->snapshot_id = 100;
+ ref->retention = SnapshotRef::Branch{};
+ base_metadata_->refs["main"] = ref;
+
+ table::AssertRefSnapshotID requirement("main", std::nullopt);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("created concurrently"));
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedFieldIdSuccess) {
+ base_metadata_->last_column_id = 10;
+ table::AssertLastAssignedFieldId requirement(10);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedFieldIdMismatch) {
+ base_metadata_->last_column_id = 10;
+ table::AssertLastAssignedFieldId requirement(15);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("last assigned field ID does not
match"));
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedFieldIdNullBase) {
+ table::AssertLastAssignedFieldId requirement(10);
+
+ EXPECT_THAT(requirement.Validate(nullptr), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedPartitionIdSuccess) {
+ base_metadata_->last_partition_id = 5;
+ table::AssertLastAssignedPartitionId requirement(5);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedPartitionIdMismatch) {
+ base_metadata_->last_partition_id = 5;
+ table::AssertLastAssignedPartitionId requirement(8);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("last assigned partition ID does not
match"));
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertLastAssignedPartitionIdNullBase) {
+ table::AssertLastAssignedPartitionId requirement(5);
+
+ auto status = requirement.Validate(nullptr);
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("metadata is missing"));
+}
+
+TEST_F(TableMetadataBuilderTest, TableRequirementAssertDefaultSpecIDSuccess) {
+ base_metadata_->default_spec_id = 3;
+ table::AssertDefaultSpecID requirement(3);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest, TableRequirementAssertDefaultSpecIDMismatch) {
+ base_metadata_->default_spec_id = 3;
+ table::AssertDefaultSpecID requirement(7);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("spec changed"));
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertDefaultSortOrderIDSuccess) {
+ base_metadata_->default_sort_order_id = 2;
+ table::AssertDefaultSortOrderID requirement(2);
+
+ ASSERT_THAT(requirement.Validate(base_metadata_.get()), IsOk());
+}
+
+TEST_F(TableMetadataBuilderTest,
TableRequirementAssertDefaultSortOrderIDMismatch) {
+ base_metadata_->default_sort_order_id = 2;
+ table::AssertDefaultSortOrderID requirement(4);
+
+ auto status = requirement.Validate(base_metadata_.get());
+ EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed));
+ EXPECT_THAT(status, HasErrorMessage("sort order changed"));
+}
+
// ============================================================================
// Integration Tests - End-to-End Workflow
// ============================================================================
