diff --git a/src/iceberg/table_requirement.cc b/src/iceberg/table_requirement.cc index 0b8e79578..3c0a01e48 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 6b2314f1e..9b9804819 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(); + 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(); + 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(); + 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 // ============================================================================