diff --git a/src/iceberg/snapshot.h b/src/iceberg/snapshot.h index d41795d53..5afe2d22e 100644 --- a/src/iceberg/snapshot.h +++ b/src/iceberg/snapshot.h @@ -62,6 +62,8 @@ ICEBERG_EXPORT constexpr Result SnapshotRefTypeFromString( /// \brief A reference to a snapshot, either a branch or a tag. struct ICEBERG_EXPORT SnapshotRef { + static constexpr std::string_view kMainBranch = "main"; + struct ICEBERG_EXPORT Branch { /// A positive number for the minimum number of snapshots to keep in a branch while /// expiring snapshots. Defaults to table property diff --git a/src/iceberg/table_requirements.cc b/src/iceberg/table_requirements.cc index 3e0aa024f..6de6c59e6 100644 --- a/src/iceberg/table_requirements.cc +++ b/src/iceberg/table_requirements.cc @@ -21,10 +21,10 @@ #include +#include "iceberg/snapshot.h" #include "iceberg/table_metadata.h" #include "iceberg/table_requirement.h" #include "iceberg/table_update.h" -#include "iceberg/util/macros.h" namespace iceberg { @@ -36,12 +36,78 @@ Result>> TableUpdateContext::Build return std::move(requirements_); } +void TableUpdateContext::RequireLastAssignedFieldIdUnchanged() { + if (!added_last_assigned_field_id_) { + if (base_ != nullptr) { + AddRequirement( + std::make_unique(base_->last_column_id)); + } + added_last_assigned_field_id_ = true; + } +} + +void TableUpdateContext::RequireCurrentSchemaIdUnchanged() { + if (!added_current_schema_id_) { + if (base_ != nullptr && !is_replace_) { + AddRequirement(std::make_unique( + base_->current_schema_id.value())); + } + added_current_schema_id_ = true; + } +} + +void TableUpdateContext::RequireLastAssignedPartitionIdUnchanged() { + if (!added_last_assigned_partition_id_) { + if (base_ != nullptr) { + AddRequirement(std::make_unique( + base_->last_partition_id)); + } + added_last_assigned_partition_id_ = true; + } +} + +void TableUpdateContext::RequireDefaultSpecIdUnchanged() { + if (!added_default_spec_id_) { + if (base_ != nullptr && !is_replace_) { + AddRequirement( + std::make_unique(base_->default_spec_id)); + } + added_default_spec_id_ = true; + } +} + +void TableUpdateContext::RequireDefaultSortOrderIdUnchanged() { + if (!added_default_sort_order_id_) { + if (base_ != nullptr && !is_replace_) { + AddRequirement(std::make_unique( + base_->default_sort_order_id)); + } + added_default_sort_order_id_ = true; + } +} + +void TableUpdateContext::RequireNoBranchesChanged() { + if (base_ != nullptr && !is_replace_) { + for (const auto& [name, ref] : base_->refs) { + if (ref->type() == SnapshotRefType::kBranch && name != SnapshotRef::kMainBranch) { + AddRequirement( + std::make_unique(name, ref->snapshot_id)); + } + } + } +} + +bool TableUpdateContext::AddChangedRef(const std::string& ref_name) { + auto [_, inserted] = changed_refs_.insert(ref_name); + return inserted; +} + Result>> TableRequirements::ForCreateTable( const std::vector>& table_updates) { TableUpdateContext context(nullptr, false); context.AddRequirement(std::make_unique()); for (const auto& update : table_updates) { - ICEBERG_RETURN_UNEXPECTED(update->GenerateRequirements(context)); + update->GenerateRequirements(context); } return context.Build(); } @@ -52,7 +118,7 @@ Result>> TableRequirements::ForRep TableUpdateContext context(&base, true); context.AddRequirement(std::make_unique(base.table_uuid)); for (const auto& update : table_updates) { - ICEBERG_RETURN_UNEXPECTED(update->GenerateRequirements(context)); + update->GenerateRequirements(context); } return context.Build(); } @@ -63,7 +129,7 @@ Result>> TableRequirements::ForUpd TableUpdateContext context(&base, false); context.AddRequirement(std::make_unique(base.table_uuid)); for (const auto& update : table_updates) { - ICEBERG_RETURN_UNEXPECTED(update->GenerateRequirements(context)); + update->GenerateRequirements(context); } return context.Build(); } diff --git a/src/iceberg/table_requirements.h b/src/iceberg/table_requirements.h index 7af2fb2df..f79f0bead 100644 --- a/src/iceberg/table_requirements.h +++ b/src/iceberg/table_requirements.h @@ -27,6 +27,8 @@ /// for optimistic concurrency control when committing table changes. #include +#include +#include #include #include "iceberg/iceberg_export.h" @@ -68,27 +70,24 @@ class ICEBERG_EXPORT TableUpdateContext { /// \brief Build and return the list of requirements Result>> Build(); - // Getters for deduplication flags - bool added_last_assigned_field_id() const { return added_last_assigned_field_id_; } - bool added_current_schema_id() const { return added_current_schema_id_; } - bool added_last_assigned_partition_id() const { - return added_last_assigned_partition_id_; - } - bool added_default_spec_id() const { return added_default_spec_id_; } - bool added_default_sort_order_id() const { return added_default_sort_order_id_; } - - // Setters for deduplication flags - void set_added_last_assigned_field_id(bool value) { - added_last_assigned_field_id_ = value; - } - void set_added_current_schema_id(bool value) { added_current_schema_id_ = value; } - void set_added_last_assigned_partition_id(bool value) { - added_last_assigned_partition_id_ = value; - } - void set_added_default_spec_id(bool value) { added_default_spec_id_ = value; } - void set_added_default_sort_order_id(bool value) { - added_default_sort_order_id_ = value; - } + // Helper methods to deduplicate requirements to add. + /// \brief Require that the last assigned field ID remains unchanged + void RequireLastAssignedFieldIdUnchanged(); + /// \brief Require that the current schema ID remains unchanged + void RequireCurrentSchemaIdUnchanged(); + /// \brief Require that the last assigned partition ID remains unchanged + void RequireLastAssignedPartitionIdUnchanged(); + /// \brief Require that the default spec ID remains unchanged + void RequireDefaultSpecIdUnchanged(); + /// \brief Require that the default sort order ID remains unchanged + void RequireDefaultSortOrderIdUnchanged(); + /// \brief Require that no branches have been changed + void RequireNoBranchesChanged(); + + /// \brief Track a changed ref and return whether it was newly added + /// \param ref_name The name of the ref being changed + /// \return true if this is the first time the ref is being changed + bool AddChangedRef(const std::string& ref_name); private: const TableMetadata* base_; @@ -102,6 +101,9 @@ class ICEBERG_EXPORT TableUpdateContext { bool added_last_assigned_partition_id_ = false; bool added_default_spec_id_ = false; bool added_default_sort_order_id_ = false; + + // Track refs that have been changed to avoid duplicate requirements + std::unordered_set changed_refs_; }; /// \brief Factory class for generating table requirements diff --git a/src/iceberg/table_update.cc b/src/iceberg/table_update.cc index 6c1ad72e4..90f7de622 100644 --- a/src/iceberg/table_update.cc +++ b/src/iceberg/table_update.cc @@ -21,7 +21,6 @@ #include "iceberg/exception.h" #include "iceberg/table_metadata.h" -#include "iceberg/table_requirement.h" #include "iceberg/table_requirements.h" namespace iceberg::table { @@ -32,9 +31,8 @@ void AssignUUID::ApplyTo(TableMetadataBuilder& builder) const { builder.AssignUUID(uuid_); } -Status AssignUUID::GenerateRequirements(TableUpdateContext& context) const { +void AssignUUID::GenerateRequirements(TableUpdateContext& context) const { // AssignUUID does not generate additional requirements. - return {}; } // UpgradeFormatVersion @@ -43,8 +41,8 @@ void UpgradeFormatVersion::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status UpgradeFormatVersion::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("UpgradeFormatVersion::GenerateRequirements not implemented"); +void UpgradeFormatVersion::GenerateRequirements(TableUpdateContext& context) const { + // UpgradeFormatVersion doesn't generate any requirements } // AddSchema @@ -53,8 +51,8 @@ void AddSchema::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status AddSchema::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("AddTableSchema::GenerateRequirements not implemented"); +void AddSchema::GenerateRequirements(TableUpdateContext& context) const { + context.RequireLastAssignedFieldIdUnchanged(); } // SetCurrentSchema @@ -63,8 +61,8 @@ void SetCurrentSchema::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status SetCurrentSchema::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("SetCurrentTableSchema::GenerateRequirements not implemented"); +void SetCurrentSchema::GenerateRequirements(TableUpdateContext& context) const { + context.RequireCurrentSchemaIdUnchanged(); } // AddPartitionSpec @@ -73,8 +71,8 @@ void AddPartitionSpec::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status AddPartitionSpec::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("AddTablePartitionSpec::GenerateRequirements not implemented"); +void AddPartitionSpec::GenerateRequirements(TableUpdateContext& context) const { + context.RequireLastAssignedPartitionIdUnchanged(); } // SetDefaultPartitionSpec @@ -83,9 +81,8 @@ void SetDefaultPartitionSpec::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status SetDefaultPartitionSpec::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented( - "SetDefaultTablePartitionSpec::GenerateRequirements not implemented"); +void SetDefaultPartitionSpec::GenerateRequirements(TableUpdateContext& context) const { + context.RequireDefaultSpecIdUnchanged(); } // RemovePartitionSpecs @@ -94,9 +91,9 @@ void RemovePartitionSpecs::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status RemovePartitionSpecs::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented( - "RemoveTablePartitionSpecs::GenerateRequirements not implemented"); +void RemovePartitionSpecs::GenerateRequirements(TableUpdateContext& context) const { + context.RequireDefaultSpecIdUnchanged(); + context.RequireNoBranchesChanged(); } // RemoveSchemas @@ -105,28 +102,29 @@ void RemoveSchemas::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status RemoveSchemas::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("RemoveTableSchemas::GenerateRequirements not implemented"); +void RemoveSchemas::GenerateRequirements(TableUpdateContext& context) const { + context.RequireCurrentSchemaIdUnchanged(); + context.RequireNoBranchesChanged(); } // AddSortOrder void AddSortOrder::ApplyTo(TableMetadataBuilder& builder) const { - throw IcebergError(std::format("{} not implemented", __FUNCTION__)); + builder.AddSortOrder(sort_order_); } -Status AddSortOrder::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("AddTableSortOrder::GenerateRequirements not implemented"); +void AddSortOrder::GenerateRequirements(TableUpdateContext& context) const { + // AddSortOrder doesn't generate any requirements } // SetDefaultSortOrder void SetDefaultSortOrder::ApplyTo(TableMetadataBuilder& builder) const { - throw IcebergError(std::format("{} not implemented", __FUNCTION__)); + builder.SetDefaultSortOrder(sort_order_id_); } -Status SetDefaultSortOrder::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("SetDefaultTableSortOrder::GenerateRequirements not implemented"); +void SetDefaultSortOrder::GenerateRequirements(TableUpdateContext& context) const { + context.RequireDefaultSortOrderIdUnchanged(); } // AddSnapshot @@ -135,16 +133,16 @@ void AddSnapshot::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status AddSnapshot::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("AddTableSnapshot::GenerateRequirements not implemented"); +void AddSnapshot::GenerateRequirements(TableUpdateContext& context) const { + // AddSnapshot doesn't generate any requirements } // RemoveSnapshots void RemoveSnapshots::ApplyTo(TableMetadataBuilder& builder) const {} -Status RemoveSnapshots::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("RemoveTableSnapshots::GenerateRequirements not implemented"); +void RemoveSnapshots::GenerateRequirements(TableUpdateContext& context) const { + // RemoveSnapshots doesn't generate any requirements } // RemoveSnapshotRef @@ -153,8 +151,8 @@ void RemoveSnapshotRef::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status RemoveSnapshotRef::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("RemoveTableSnapshotRef::GenerateRequirements not implemented"); +void RemoveSnapshotRef::GenerateRequirements(TableUpdateContext& context) const { + // RemoveSnapshotRef doesn't generate any requirements } // SetSnapshotRef @@ -163,8 +161,17 @@ void SetSnapshotRef::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status SetSnapshotRef::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("SetTableSnapshotRef::GenerateRequirements not implemented"); +void SetSnapshotRef::GenerateRequirements(TableUpdateContext& context) const { + bool added = context.AddChangedRef(ref_name_); + if (added && context.base() != nullptr && !context.is_replace()) { + const auto& refs = context.base()->refs; + auto it = refs.find(ref_name_); + // Require that the ref does not exist (nullopt) or is the same as the base snapshot + std::optional base_snapshot_id = + (it != refs.end()) ? std::make_optional(it->second->snapshot_id) : std::nullopt; + context.AddRequirement( + std::make_unique(ref_name_, base_snapshot_id)); + } } // SetProperties @@ -173,9 +180,8 @@ void SetProperties::ApplyTo(TableMetadataBuilder& builder) const { builder.SetProperties(updated_); } -Status SetProperties::GenerateRequirements(TableUpdateContext& context) const { - // No requirements - return {}; +void SetProperties::GenerateRequirements(TableUpdateContext& context) const { + // SetProperties doesn't generate any requirements } // RemoveProperties @@ -184,9 +190,8 @@ void RemoveProperties::ApplyTo(TableMetadataBuilder& builder) const { builder.RemoveProperties(removed_); } -Status RemoveProperties::GenerateRequirements(TableUpdateContext& context) const { - // No requirements - return {}; +void RemoveProperties::GenerateRequirements(TableUpdateContext& context) const { + // RemoveProperties doesn't generate any requirements } // SetLocation @@ -195,8 +200,8 @@ void SetLocation::ApplyTo(TableMetadataBuilder& builder) const { throw IcebergError(std::format("{} not implemented", __FUNCTION__)); } -Status SetLocation::GenerateRequirements(TableUpdateContext& context) const { - return NotImplemented("SetTableLocation::GenerateRequirements not implemented"); +void SetLocation::GenerateRequirements(TableUpdateContext& context) const { + // SetLocation doesn't generate any requirements } } // namespace iceberg::table diff --git a/src/iceberg/table_update.h b/src/iceberg/table_update.h index 445295a4d..93b48cf27 100644 --- a/src/iceberg/table_update.h +++ b/src/iceberg/table_update.h @@ -58,8 +58,7 @@ class ICEBERG_EXPORT TableUpdate { /// provides information about the base metadata and operation mode. /// /// \param context The context containing base metadata and operation state - /// \return Status indicating success or failure with error details - virtual Status GenerateRequirements(TableUpdateContext& context) const = 0; + virtual void GenerateRequirements(TableUpdateContext& context) const = 0; }; namespace table { @@ -73,7 +72,7 @@ class ICEBERG_EXPORT AssignUUID : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::string uuid_; @@ -89,7 +88,7 @@ class ICEBERG_EXPORT UpgradeFormatVersion : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: int8_t format_version_; @@ -107,7 +106,7 @@ class ICEBERG_EXPORT AddSchema : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::shared_ptr schema_; @@ -123,7 +122,7 @@ class ICEBERG_EXPORT SetCurrentSchema : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: int32_t schema_id_; @@ -139,7 +138,7 @@ class ICEBERG_EXPORT AddPartitionSpec : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::shared_ptr spec_; @@ -154,7 +153,7 @@ class ICEBERG_EXPORT SetDefaultPartitionSpec : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: int32_t spec_id_; @@ -170,7 +169,7 @@ class ICEBERG_EXPORT RemovePartitionSpecs : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::vector spec_ids_; @@ -186,7 +185,7 @@ class ICEBERG_EXPORT RemoveSchemas : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::vector schema_ids_; @@ -202,7 +201,7 @@ class ICEBERG_EXPORT AddSortOrder : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::shared_ptr sort_order_; @@ -217,7 +216,7 @@ class ICEBERG_EXPORT SetDefaultSortOrder : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: int32_t sort_order_id_; @@ -233,7 +232,7 @@ class ICEBERG_EXPORT AddSnapshot : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::shared_ptr snapshot_; @@ -249,7 +248,7 @@ class ICEBERG_EXPORT RemoveSnapshots : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::vector snapshot_ids_; @@ -264,7 +263,7 @@ class ICEBERG_EXPORT RemoveSnapshotRef : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::string ref_name_; @@ -297,7 +296,7 @@ class ICEBERG_EXPORT SetSnapshotRef : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::string ref_name_; @@ -318,7 +317,7 @@ class ICEBERG_EXPORT SetProperties : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::unordered_map updated_; @@ -334,7 +333,7 @@ class ICEBERG_EXPORT RemoveProperties : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::vector removed_; @@ -349,7 +348,7 @@ class ICEBERG_EXPORT SetLocation : public TableUpdate { void ApplyTo(TableMetadataBuilder& builder) const override; - Status GenerateRequirements(TableUpdateContext& context) const override; + void GenerateRequirements(TableUpdateContext& context) const override; private: std::string location_; diff --git a/src/iceberg/test/table_requirements_test.cc b/src/iceberg/test/table_requirements_test.cc index 441e72d41..041b44dd1 100644 --- a/src/iceberg/test/table_requirements_test.cc +++ b/src/iceberg/test/table_requirements_test.cc @@ -20,18 +20,21 @@ #include "iceberg/table_requirements.h" #include +#include #include #include #include #include "iceberg/partition_spec.h" +#include "iceberg/schema.h" #include "iceberg/snapshot.h" #include "iceberg/sort_order.h" #include "iceberg/table_metadata.h" #include "iceberg/table_requirement.h" #include "iceberg/table_update.h" #include "iceberg/test/matchers.h" +#include "iceberg/type.h" namespace iceberg { @@ -47,16 +50,43 @@ std::unique_ptr CreateBaseMetadata( metadata->last_sequence_number = 0; metadata->last_updated_ms = TimePointMs{std::chrono::milliseconds(1000)}; metadata->last_column_id = 0; + metadata->current_schema_id = Schema::kInitialSchemaId; metadata->default_spec_id = PartitionSpec::kInitialSpecId; metadata->last_partition_id = 0; metadata->current_snapshot_id = Snapshot::kInvalidSnapshotId; - metadata->default_sort_order_id = SortOrder::kInitialSortOrderId; + metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; metadata->next_row_id = TableMetadata::kInitialRowId; return metadata; } +// Helper function to create a simple schema for tests +std::shared_ptr CreateTestSchema(int32_t schema_id = 0) { + std::vector fields; + fields.emplace_back(SchemaField::MakeRequired(1, "id", int32())); + return std::make_shared(std::move(fields), schema_id); +} + +// Helper function to count requirements of a specific type +template +int CountRequirementsOfType( + const std::vector>& requirements) { + return std::ranges::count_if(requirements, [](const auto& req) { + return dynamic_cast(req.get()) != nullptr; + }); +} + +// Helper function to add a branch to metadata +void AddBranch(TableMetadata& metadata, const std::string& name, int64_t snapshot_id) { + auto ref = std::make_shared(); + ref->snapshot_id = snapshot_id; + ref->retention = SnapshotRef::Branch{}; + metadata.refs[name] = ref; +} + } // namespace +// Empty Updates Tests + TEST(TableRequirementsTest, EmptyUpdatesForCreateTable) { std::vector> updates; @@ -104,6 +134,8 @@ TEST(TableRequirementsTest, EmptyUpdatesForReplaceTable) { EXPECT_EQ(assert_uuid->uuid(), metadata->table_uuid); } +// Table Existence Tests + TEST(TableRequirementsTest, TableAlreadyExists) { std::vector> updates; @@ -134,7 +166,8 @@ TEST(TableRequirementsTest, TableDoesNotExist) { EXPECT_THAT(status, IsOk()); } -// Test for AssignUUID update +// AssignUUID Tests + TEST(TableRequirementsTest, AssignUUID) { auto metadata = CreateBaseMetadata("original-uuid"); std::vector> updates; @@ -207,4 +240,875 @@ TEST(TableRequirementsTest, AssignUUIDForReplaceTable) { EXPECT_THAT(status, IsOk()); } +// UpgradeFormatVersion Tests + +TEST(TableRequirementsTest, UpgradeFormatVersion) { + auto metadata = CreateBaseMetadata(); + std::vector> updates; + updates.push_back(std::make_unique(2)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // UpgradeFormatVersion doesn't add additional requirements + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// AddSchema Tests + +TEST(TableRequirementsTest, AddSchema) { + auto metadata = CreateBaseMetadata(); + metadata->last_column_id = 1; + std::vector> updates; + + auto schema = CreateTestSchema(); + // Add multiple AddSchema updates + updates.push_back(std::make_unique(schema, 1)); + updates.push_back(std::make_unique(schema, 1)); + updates.push_back(std::make_unique(schema, 1)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertLastAssignedFieldId (deduplicated) + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the last assigned field ID value + auto* assert_field_id = + dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_field_id, nullptr); + EXPECT_EQ(assert_field_id->last_assigned_field_id(), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, AddSchemaFailure) { + auto metadata = CreateBaseMetadata(); + metadata->last_column_id = 2; + + std::vector> updates; + auto schema = CreateTestSchema(); + updates.push_back(std::make_unique(schema, 2)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different last_column_id + auto updated = CreateBaseMetadata(); + updated->last_column_id = 3; + + // Find and validate the AssertLastAssignedFieldId requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("last assigned field ID does not match")); + break; + } + } +} + +// SetCurrentSchema Tests + +TEST(TableRequirementsTest, SetCurrentSchema) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + std::vector> updates; + + // Add multiple SetCurrentSchema updates + updates.push_back(std::make_unique(3)); + updates.push_back(std::make_unique(4)); + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertCurrentSchemaID (deduplicated) + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the current schema ID value + auto* assert_schema_id = + dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_schema_id, nullptr); + EXPECT_EQ(assert_schema_id->schema_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, SetCurrentSchemaFailure) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(3)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different current_schema_id + auto updated = CreateBaseMetadata(); + updated->current_schema_id = 4; + + // Find and validate the AssertCurrentSchemaID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("current schema ID does not match")); + break; + } + } +} + +// AddPartitionSpec Tests + +TEST(TableRequirementsTest, AddPartitionSpec) { + auto metadata = CreateBaseMetadata(); + metadata->last_partition_id = 3; + + std::vector> updates; + updates.push_back( + std::make_unique(PartitionSpec::Unpartitioned())); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertLastAssignedPartitionId + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), + 1); + + // Verify the last assigned partition ID value + auto* assert_partition_id = + dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_partition_id, nullptr); + EXPECT_EQ(assert_partition_id->last_assigned_partition_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, AddPartitionSpecFailure) { + auto metadata = CreateBaseMetadata(); + metadata->last_partition_id = 3; + + std::vector> updates; + updates.push_back( + std::make_unique(PartitionSpec::Unpartitioned())); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different last_partition_id + auto updated = CreateBaseMetadata(); + updated->last_partition_id = 4; + + // Find and validate the AssertLastAssignedPartitionId requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("last assigned partition ID does not match")); + break; + } + } +} + +// SetDefaultPartitionSpec Tests + +TEST(TableRequirementsTest, SetDefaultPartitionSpec) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + + std::vector> updates; + // Add multiple SetDefaultPartitionSpec updates + updates.push_back(std::make_unique(3)); + updates.push_back(std::make_unique(4)); + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertDefaultSpecID (deduplicated) + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the default spec ID value + auto* assert_spec_id = dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_spec_id, nullptr); + EXPECT_EQ(assert_spec_id->spec_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, SetDefaultPartitionSpecFailure) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = PartitionSpec::kInitialSpecId; + + std::vector> updates; + updates.push_back( + std::make_unique(PartitionSpec::kInitialSpecId)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different default_spec_id + auto updated = CreateBaseMetadata(); + updated->default_spec_id = PartitionSpec::kInitialSpecId + 1; + + // Find and validate the AssertDefaultSpecID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("default partition spec changed")); + break; + } + } +} + +// RemovePartitionSpecs Tests + +TEST(TableRequirementsTest, RemovePartitionSpecs) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + + std::vector> updates; + updates.push_back( + std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertDefaultSpecID + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the default spec ID value + auto* assert_spec_id = dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_spec_id, nullptr); + EXPECT_EQ(assert_spec_id->spec_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, RemovePartitionSpecsWithBranch) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + AddBranch(*metadata, "branch", 42); + + std::vector> updates; + updates.push_back( + std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertDefaultSpecID + AssertRefSnapshotID + ASSERT_EQ(requirements.size(), 3); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, RemovePartitionSpecsWithSpecChangedFailure) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + + std::vector> updates; + updates.push_back( + std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different default_spec_id + auto updated = CreateBaseMetadata(); + updated->default_spec_id = 4; + + // Find and validate the AssertDefaultSpecID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("default partition spec changed")); + break; + } + } +} + +TEST(TableRequirementsTest, RemovePartitionSpecsWithBranchChangedFailure) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + AddBranch(*metadata, "test", 42); + + std::vector> updates; + updates.push_back( + std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with changed branch + auto updated = CreateBaseMetadata(); + updated->default_spec_id = 3; + AddBranch(*updated, "test", 43); + + // Find and validate the AssertRefSnapshotID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("has changed")); + break; + } + } +} + +// RemoveSchemas Tests + +TEST(TableRequirementsTest, RemoveSchemas) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertCurrentSchemaID + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the current schema ID value + auto* assert_schema_id = + dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_schema_id, nullptr); + EXPECT_EQ(assert_schema_id->schema_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, RemoveSchemasWithBranch) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + AddBranch(*metadata, "branch", 42); + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertCurrentSchemaID + AssertRefSnapshotID + ASSERT_EQ(requirements.size(), 3); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, RemoveSchemasWithSchemaChangedFailure) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different current_schema_id + auto updated = CreateBaseMetadata(); + updated->current_schema_id = 4; + + // Find and validate the AssertCurrentSchemaID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("current schema ID does not match")); + break; + } + } +} + +TEST(TableRequirementsTest, RemoveSchemasWithBranchChangedFailure) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + AddBranch(*metadata, "test", 42); + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with changed branch + auto updated = CreateBaseMetadata(); + updated->current_schema_id = 3; + AddBranch(*updated, "test", 43); + + // Find and validate the AssertRefSnapshotID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("has changed")); + break; + } + } +} + +// AddSortOrder Tests + +TEST(TableRequirementsTest, AddSortOrder) { + auto metadata = CreateBaseMetadata(); + std::vector> updates; + + updates.push_back(std::make_unique(SortOrder::Unsorted())); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // AddSortOrder doesn't add additional requirements + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// SetDefaultSortOrder Tests + +TEST(TableRequirementsTest, SetDefaultSortOrder) { + auto metadata = CreateBaseMetadata(); + metadata->default_sort_order_id = 3; + + std::vector> updates; + // Add multiple SetDefaultSortOrder updates + updates.push_back(std::make_unique(3)); + updates.push_back(std::make_unique(4)); + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have AssertUUID + AssertDefaultSortOrderID (deduplicated) + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Verify the default sort order ID value + auto* assert_sort_order_id = + dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_sort_order_id, nullptr); + EXPECT_EQ(assert_sort_order_id->sort_order_id(), 3); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, SetDefaultSortOrderFailure) { + auto metadata = CreateBaseMetadata(); + metadata->default_sort_order_id = SortOrder::kUnsortedOrderId; + + std::vector> updates; + updates.push_back( + std::make_unique(SortOrder::kUnsortedOrderId)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + + // Create updated metadata with different default_sort_order_id + auto updated = CreateBaseMetadata(); + updated->default_sort_order_id = SortOrder::kUnsortedOrderId + 1; + + // Find and validate the AssertDefaultSortOrderID requirement + for (const auto& req : requirements) { + if (dynamic_cast(req.get()) != nullptr) { + auto status = req->Validate(updated.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("default sort order changed")); + break; + } + } +} + +// AddSnapshot Tests + +TEST(TableRequirementsTest, AddSnapshot) { + auto metadata = CreateBaseMetadata(); + + std::vector> updates; + auto snapshot = std::make_shared(); + snapshot->snapshot_id = 1; + snapshot->sequence_number = 1; + snapshot->timestamp_ms = TimePointMs{std::chrono::milliseconds(1000)}; + snapshot->manifest_list = "s3://bucket/manifest_list"; + updates.push_back(std::make_unique(snapshot)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// RemoveSnapshots Tests + +TEST(TableRequirementsTest, RemoveSnapshots) { + auto metadata = CreateBaseMetadata(); + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{0})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// SetSnapshotRef Tests + +TEST(TableRequirementsTest, SetSnapshotRef) { + constexpr int64_t kSnapshotId = 14; + const std::string kRefName = "branch"; + + auto metadata = CreateBaseMetadata(); + AddBranch(*metadata, kRefName, kSnapshotId); + + // Multiple updates to same ref should deduplicate + std::vector> updates; + updates.push_back(std::make_unique(kRefName, kSnapshotId, + SnapshotRefType::kBranch)); + updates.push_back(std::make_unique(kRefName, kSnapshotId + 1, + SnapshotRefType::kBranch)); + updates.push_back(std::make_unique(kRefName, kSnapshotId + 2, + SnapshotRefType::kBranch)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } + + ASSERT_EQ(requirements.size(), 2); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + auto* assert_ref = dynamic_cast(requirements[1].get()); + ASSERT_NE(assert_ref, nullptr); + EXPECT_EQ(assert_ref->snapshot_id(), kSnapshotId); + EXPECT_EQ(assert_ref->ref_name(), kRefName); +} + +// RemoveSnapshotRef Tests + +TEST(TableRequirementsTest, RemoveSnapshotRef) { + auto metadata = CreateBaseMetadata(); + + std::vector> updates; + updates.push_back(std::make_unique("branch")); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// SetAndRemoveProperties Tests + +TEST(TableRequirementsTest, SetProperties) { + auto metadata = CreateBaseMetadata(); + std::vector> updates; + + std::unordered_map props; + props["test"] = "value"; + updates.push_back(std::make_unique(props)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // SetProperties doesn't add additional requirements + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +TEST(TableRequirementsTest, RemoveProperties) { + auto metadata = CreateBaseMetadata(); + std::vector> updates; + + updates.push_back( + std::make_unique(std::vector{"test"})); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // RemoveProperties doesn't add additional requirements + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// SetLocation Tests + +TEST(TableRequirementsTest, SetLocation) { + auto metadata = CreateBaseMetadata(); + std::vector> updates; + + updates.push_back(std::make_unique("s3://new-bucket/test")); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // SetLocation doesn't add additional requirements + ASSERT_EQ(requirements.size(), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + + // Validate against base metadata + for (const auto& req : requirements) { + EXPECT_THAT(req->Validate(metadata.get()), IsOk()); + } +} + +// AssertRefSnapshotID Tests + +TEST(TableRequirementsTest, AssertRefSnapshotIDSuccess) { + auto metadata = CreateBaseMetadata(); + AddBranch(*metadata, "branch", 14); + + table::AssertRefSnapshotID requirement("branch", 14); + auto status = requirement.Validate(metadata.get()); + EXPECT_THAT(status, IsOk()); +} + +TEST(TableRequirementsTest, AssertRefSnapshotIDCreatedConcurrently) { + auto metadata = CreateBaseMetadata(); + AddBranch(*metadata, "random_branch", 14); + + // Requirement expects ref doesn't exist (nullopt snapshot_id) + table::AssertRefSnapshotID requirement("random_branch", std::nullopt); + auto status = requirement.Validate(metadata.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("was created concurrently")); +} + +TEST(TableRequirementsTest, AssertRefSnapshotIDMissing) { + auto metadata = CreateBaseMetadata(); + // No branch added + + // Requirement expects a snapshot ID that doesn't exist + table::AssertRefSnapshotID requirement("random_branch", 14); + auto status = requirement.Validate(metadata.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("is missing")); +} + +TEST(TableRequirementsTest, AssertRefSnapshotIDChanged) { + auto metadata = CreateBaseMetadata(); + AddBranch(*metadata, "random_branch", 15); + + // Requirement expects snapshot ID 14, but actual is 15 + table::AssertRefSnapshotID requirement("random_branch", 14); + auto status = requirement.Validate(metadata.get()); + EXPECT_THAT(status, IsError(ErrorKind::kCommitFailed)); + EXPECT_THAT(status, HasErrorMessage("has changed")); +} + +// Replace Table Tests (less restrictive than Update) + +TEST(TableRequirementsTest, ReplaceTableDoesNotRequireCurrentSchemaID) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForReplaceTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Replace table should NOT add AssertCurrentSchemaID + EXPECT_EQ(CountRequirementsOfType(requirements), 0); +} + +TEST(TableRequirementsTest, ReplaceTableDoesNotRequireDefaultSpecID) { + auto metadata = CreateBaseMetadata(); + metadata->default_spec_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForReplaceTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Replace table should NOT add AssertDefaultSpecID + EXPECT_EQ(CountRequirementsOfType(requirements), 0); +} + +TEST(TableRequirementsTest, ReplaceTableDoesNotRequireDefaultSortOrderID) { + auto metadata = CreateBaseMetadata(); + metadata->default_sort_order_id = 3; + + std::vector> updates; + updates.push_back(std::make_unique(5)); + + auto result = TableRequirements::ForReplaceTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Replace table should NOT add AssertDefaultSortOrderID + EXPECT_EQ(CountRequirementsOfType(requirements), 0); +} + +TEST(TableRequirementsTest, ReplaceTableDoesNotAddBranchRequirements) { + auto metadata = CreateBaseMetadata(); + metadata->current_schema_id = 3; + AddBranch(*metadata, "branch", 42); + + std::vector> updates; + updates.push_back(std::make_unique(std::vector{1, 2})); + + auto result = TableRequirements::ForReplaceTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Replace table should NOT add AssertRefSnapshotID for branches + EXPECT_EQ(CountRequirementsOfType(requirements), 0); +} + +// Combined Updates Tests + +TEST(TableRequirementsTest, MultipleUpdatesDeduplication) { + auto metadata = CreateBaseMetadata(); + metadata->last_column_id = 1; + metadata->current_schema_id = 0; + + std::vector> updates; + auto schema = CreateTestSchema(); + // Add multiple AddSchema updates - should only generate one requirement + updates.push_back(std::make_unique(schema, 1)); + updates.push_back(std::make_unique(schema, 1)); + // Add multiple SetCurrentSchema updates - should only generate one requirement + updates.push_back(std::make_unique(0)); + updates.push_back(std::make_unique(1)); + + auto result = TableRequirements::ForUpdateTable(*metadata, updates); + ASSERT_THAT(result, IsOk()); + + auto& requirements = result.value(); + // Should have: 1 AssertUUID + 1 AssertLastAssignedFieldId + 1 AssertCurrentSchemaID + ASSERT_EQ(requirements.size(), 3); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); + EXPECT_EQ(CountRequirementsOfType(requirements), 1); +} + } // namespace iceberg diff --git a/src/iceberg/test/table_update_test.cc b/src/iceberg/test/table_update_test.cc index 298141a93..d250b6d20 100644 --- a/src/iceberg/test/table_update_test.cc +++ b/src/iceberg/test/table_update_test.cc @@ -19,6 +19,7 @@ #include "iceberg/table_update.h" +#include #include #include #include @@ -26,23 +27,34 @@ #include #include "iceberg/partition_spec.h" +#include "iceberg/schema.h" #include "iceberg/snapshot.h" +#include "iceberg/sort_field.h" #include "iceberg/sort_order.h" #include "iceberg/table_metadata.h" #include "iceberg/table_requirement.h" #include "iceberg/table_requirements.h" #include "iceberg/test/matchers.h" +#include "iceberg/transform.h" +#include "iceberg/type.h" namespace iceberg { namespace { +// Helper function to create a simple schema for testing +std::shared_ptr CreateTestSchema() { + auto field1 = SchemaField::MakeRequired(1, "id", int32()); + auto field2 = SchemaField::MakeRequired(2, "data", string()); + auto field3 = SchemaField::MakeRequired(3, "ts", timestamp()); + return std::make_shared(std::vector{field1, field2, field3}, 0); +} + // Helper function to generate requirements std::vector> GenerateRequirements( const TableUpdate& update, const TableMetadata* base) { TableUpdateContext context(base, /*is_replace=*/false); - EXPECT_THAT(update.GenerateRequirements(context), IsOk()); - + update.GenerateRequirements(context), IsOk(); auto requirements = context.Build(); EXPECT_THAT(requirements, IsOk()); return std::move(requirements.value()); @@ -56,35 +68,250 @@ std::unique_ptr CreateBaseMetadata() { metadata->location = "s3://bucket/test"; metadata->last_sequence_number = 0; metadata->last_updated_ms = TimePointMs{std::chrono::milliseconds(1000)}; - metadata->last_column_id = 0; + metadata->last_column_id = 3; + metadata->current_schema_id = 0; + metadata->schemas.push_back(CreateTestSchema()); metadata->default_spec_id = PartitionSpec::kInitialSpecId; metadata->last_partition_id = 0; metadata->current_snapshot_id = Snapshot::kInvalidSnapshotId; metadata->default_sort_order_id = SortOrder::kInitialSortOrderId; + metadata->sort_orders.push_back(SortOrder::Unsorted()); metadata->next_row_id = TableMetadata::kInitialRowId; return metadata; } } // namespace -// Test GenerateRequirements for AssignUUID update -TEST(TableUpdateTest, AssignUUIDGenerateRequirements) { - table::AssignUUID update("new-uuid"); +// Parameter struct for testing GenerateRequirements behavior +struct GenerateRequirementsTestParam { + std::string test_name; + std::function()> update_factory; + // Expected number of requirements for existing table (new table always expects 0) + size_t expected_existing_table_count; + // Optional validator function to check the generated requirements + std::function>&, + const TableMetadata*)> + validator; +}; - // New table - no requirements (AssignUUID doesn't generate requirements) - auto new_table_reqs = GenerateRequirements(update, nullptr); +class GenerateRequirementsTest + : public ::testing::TestWithParam {}; + +TEST_P(GenerateRequirementsTest, GeneratesExpectedRequirements) { + const auto& param = GetParam(); + auto update = param.update_factory(); + + // New table - always no requirements + auto new_table_reqs = GenerateRequirements(*update, nullptr); EXPECT_TRUE(new_table_reqs.empty()); - // Existing table - AssignUUID doesn't generate requirements anymore - // The UUID assertion is added by ForUpdateTable/ForReplaceTable methods + // Existing table - check expected count + auto base = CreateBaseMetadata(); + auto existing_table_reqs = GenerateRequirements(*update, base.get()); + ASSERT_EQ(existing_table_reqs.size(), param.expected_existing_table_count); + + // Validate the requirements if validator is provided + if (param.validator) { + param.validator(existing_table_reqs, base.get()); + } +} + +INSTANTIATE_TEST_SUITE_P( + TableUpdateGenerateRequirements, GenerateRequirementsTest, + ::testing::Values( + // Updates that generate no requirements + GenerateRequirementsTestParam{ + .test_name = "AssignUUID", + .update_factory = + [] { return std::make_unique("new-uuid"); }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "UpgradeFormatVersion", + .update_factory = + [] { return std::make_unique(3); }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "AddSortOrder", + .update_factory = + [] { + auto schema = CreateTestSchema(); + SortField sort_field(1, Transform::Identity(), + SortDirection::kAscending, NullOrder::kFirst); + auto sort_order = + SortOrder::Make(*schema, 1, std::vector{sort_field}) + .value(); + return std::make_unique(std::move(sort_order)); + }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{.test_name = "AddSnapshot", + .update_factory = + [] { + auto snapshot = std::make_shared(); + return std::make_unique( + snapshot); + }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "RemoveSnapshotRef", + .update_factory = + [] { return std::make_unique("my-branch"); }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "SetProperties", + .update_factory = + [] { + return std::make_unique( + std::unordered_map{{"key", "value"}}); + }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "RemoveProperties", + .update_factory = + [] { + return std::make_unique( + std::vector{"key"}); + }, + .expected_existing_table_count = 0, + .validator = nullptr}, + GenerateRequirementsTestParam{ + .test_name = "SetLocation", + .update_factory = + [] { return std::make_unique("s3://new/location"); }, + .expected_existing_table_count = 0, + .validator = nullptr}, + + // Updates that generate single requirement for existing tables + GenerateRequirementsTestParam{ + .test_name = "AddSchema", + .update_factory = + [] { + auto new_schema = std::make_shared( + std::vector{ + SchemaField::MakeRequired(4, "new_col", string())}, + 3); + return std::make_unique(new_schema, 3); + }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = dynamic_cast( + reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->last_assigned_field_id(), base->last_column_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "SetCurrentSchema", + .update_factory = [] { return std::make_unique(1); }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = + dynamic_cast(reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->schema_id(), base->current_schema_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "AddPartitionSpec", + .update_factory = + [] { + PartitionField partition_field(1, 1, "id_identity", + Transform::Identity()); + auto spec = std::shared_ptr( + PartitionSpec::Make(1, {partition_field}).value().release()); + return std::make_unique(spec); + }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = + dynamic_cast( + reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->last_assigned_partition_id(), + base->last_partition_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "SetDefaultPartitionSpec", + .update_factory = + [] { return std::make_unique(1); }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = + dynamic_cast(reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->spec_id(), base->default_spec_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "SetDefaultSortOrder", + .update_factory = + [] { return std::make_unique(1); }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_sort_order = + dynamic_cast(reqs[0].get()); + ASSERT_NE(assert_sort_order, nullptr); + EXPECT_EQ(assert_sort_order->sort_order_id(), + base->default_sort_order_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "RemovePartitionSpecs", + .update_factory = + [] { + return std::make_unique( + std::vector{1}); + }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = + dynamic_cast(reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->spec_id(), base->default_spec_id); + }}, + GenerateRequirementsTestParam{ + .test_name = "RemoveSchemas", + .update_factory = + [] { + return std::make_unique(std::vector{1}); + }, + .expected_existing_table_count = 1, + .validator = + [](const std::vector>& reqs, + const TableMetadata* base) { + auto* assert_id = + dynamic_cast(reqs[0].get()); + ASSERT_NE(assert_id, nullptr); + EXPECT_EQ(assert_id->schema_id(), base->current_schema_id); + }}), + [](const testing::TestParamInfo& info) { + return info.param.test_name; + }); + +// Test AssignUUID ApplyTo +TEST(TableUpdateTest, AssignUUIDApplyUpdate) { auto base = CreateBaseMetadata(); - auto existing_table_reqs = GenerateRequirements(update, base.get()); - EXPECT_TRUE(existing_table_reqs.empty()); + auto builder = TableMetadataBuilder::BuildFrom(base.get()); + + // Apply AssignUUID update + table::AssignUUID uuid_update("apply-uuid"); + uuid_update.ApplyTo(*builder); - // Existing table with empty UUID - no requirements - base->table_uuid = ""; - auto empty_uuid_reqs = GenerateRequirements(update, base.get()); - EXPECT_TRUE(empty_uuid_reqs.empty()); + ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build()); + EXPECT_EQ(metadata->table_uuid, "apply-uuid"); } } // namespace iceberg diff --git a/src/iceberg/transaction.h b/src/iceberg/transaction.h index 0bcedd6d8..72ba5182c 100644 --- a/src/iceberg/transaction.h +++ b/src/iceberg/transaction.h @@ -21,7 +21,6 @@ #pragma once #include -#include #include "iceberg/iceberg_export.h" #include "iceberg/result.h"