diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 49212916a..6f25d0c9b 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -46,11 +46,11 @@ jobs: persist-credentials: false - name: Initialize CodeQL - uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2 + uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4 with: languages: actions - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2 + uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4 with: category: "/language:actions" diff --git a/meson.build b/meson.build index 68dcaa0e4..69ef47a25 100644 --- a/meson.build +++ b/meson.build @@ -18,7 +18,7 @@ project( 'iceberg', 'cpp', - version: '0.2.0', + version: '0.3.0', license: 'Apache-2.0', meson_version: '>=1.3.0', default_options: [ diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index c4e193b89..145cafe50 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -45,8 +45,10 @@ set(ICEBERG_SOURCES location_provider.cc manifest/manifest_adapter.cc manifest/manifest_entry.cc + manifest/manifest_filter_manager.cc manifest/manifest_group.cc manifest/manifest_list.cc + manifest/manifest_merge_manager.cc manifest/manifest_reader.cc manifest/manifest_util.cc manifest/manifest_writer.cc @@ -86,6 +88,7 @@ set(ICEBERG_SOURCES type.cc update/expire_snapshots.cc update/fast_append.cc + update/merging_snapshot_update.cc update/pending_update.cc update/set_snapshot.cc update/snapshot_manager.cc @@ -161,6 +164,7 @@ add_iceberg_lib(iceberg set(ICEBERG_DATA_SOURCES data/data_writer.cc + data/delete_filter.cc data/delete_loader.cc data/equality_delete_writer.cc data/position_delete_writer.cc diff --git a/src/iceberg/arrow/arrow_io.cc b/src/iceberg/arrow/arrow_io.cc index a515f3385..45ad4259e 100644 --- a/src/iceberg/arrow/arrow_io.cc +++ b/src/iceberg/arrow/arrow_io.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -568,6 +569,18 @@ Status ArrowFileSystemFileIO::DeleteFile(const std::string& file_location) { return {}; } +Status ArrowFileSystemFileIO::DeleteFiles( + const std::vector& file_locations) { + std::vector paths; + paths.reserve(file_locations.size()); + for (const auto& file_location : file_locations) { + ICEBERG_ASSIGN_OR_RAISE(auto path, ResolvePath(file_location)); + paths.push_back(std::move(path)); + } + ICEBERG_ARROW_RETURN_NOT_OK(arrow_fs_->DeleteFiles(paths)); + return {}; +} + std::unique_ptr ArrowFileSystemFileIO::MakeMockFileIO() { return std::make_unique( std::make_shared<::arrow::fs::internal::MockFileSystem>( diff --git a/src/iceberg/arrow/arrow_io_internal.h b/src/iceberg/arrow/arrow_io_internal.h index 4f170a8a4..a6b85b6c9 100644 --- a/src/iceberg/arrow/arrow_io_internal.h +++ b/src/iceberg/arrow/arrow_io_internal.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -77,6 +78,9 @@ class ICEBERG_BUNDLE_EXPORT ArrowFileSystemFileIO : public FileIO { /// \brief Delete a file at the given location. Status DeleteFile(const std::string& file_location) override; + /// \brief Delete files at the given locations. + Status DeleteFiles(const std::vector& file_locations) override; + /// \brief Get the Arrow file system. const std::shared_ptr<::arrow::fs::FileSystem>& fs() const { return arrow_fs_; } diff --git a/src/iceberg/data/delete_filter.cc b/src/iceberg/data/delete_filter.cc new file mode 100644 index 000000000..876d644e5 --- /dev/null +++ b/src/iceberg/data/delete_filter.cc @@ -0,0 +1,790 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/data/delete_filter.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/metadata_columns.h" +#include "iceberg/result.h" +#include "iceberg/row/arrow_array_wrapper.h" +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/table_metadata.h" +#include "iceberg/type.h" +#include "iceberg/util/checked_cast.h" +#include "iceberg/util/macros.h" +#include "iceberg/util/struct_like_set.h" + +namespace iceberg { + +namespace { + +std::optional FindFieldIndexById(std::span fields, + int32_t field_id) { + for (size_t pos = 0; pos < fields.size(); ++pos) { + if (fields[pos].field_id() == field_id) { + return pos; + } + } + return std::nullopt; +} + +Result RequireFieldIndexById(std::span fields, + int32_t field_id, std::string_view context) { + auto pos = FindFieldIndexById(fields, field_id); + if (pos.has_value()) { + return pos.value(); + } + return InvalidSchema("Cannot find field id {} in {}", field_id, context); +} + +// Views a source row through the equality-delete key schema: fields are selected by +// field id, then exposed by position so StructLikeSet can compare only delete keys. +class ProjectedStructLike : public StructLike { + public: + struct ProjectedField; + using ProjectedSubFields = std::vector; + + struct ProjectedField { + int32_t field_id; + size_t source_field_pos; + std::shared_ptr nested_projected_fields; + }; + + explicit ProjectedStructLike(std::shared_ptr projected_fields) + : projected_fields_(std::move(projected_fields)) { + nested_projected_structs_.reserve(projected_fields_->size()); + for (const auto& projected_field : *projected_fields_) { + nested_projected_structs_.push_back( + projected_field.nested_projected_fields == nullptr + ? nullptr + : std::make_shared( + projected_field.nested_projected_fields)); + } + } + + /// \brief Build field-id based positions from the source row to the equality keys. + /// + /// \param source_type the schema of wrapped rows + /// \param target_type the key schema used by the equality-delete set + static Result> BuildProjection( + const StructType& source_type, const StructType& target_type) { + ProjectedSubFields projected_fields; + projected_fields.reserve(target_type.fields().size()); + for (const auto& target_field : target_type.fields()) { + ICEBERG_ASSIGN_OR_RAISE( + auto source_field_pos, + RequireFieldIndexById(source_type.fields(), target_field.field_id(), + "source projection")); + const auto& source_field = source_type.fields()[source_field_pos]; + + std::shared_ptr nested_projected_fields; + if (*source_field.type() != *target_field.type()) { + if (target_field.type()->type_id() == TypeId::kStruct && + source_field.type()->type_id() == TypeId::kStruct) { + ICEBERG_ASSIGN_OR_RAISE( + nested_projected_fields, + BuildProjection( + internal::checked_cast(*source_field.type()), + internal::checked_cast(*target_field.type()))); + } else if (target_field.type()->is_nested()) { + return NotSupported("Cannot project partial non-struct equality field id {}", + target_field.field_id()); + } + } + + projected_fields.push_back(ProjectedField{ + .field_id = target_field.field_id(), + .source_field_pos = source_field_pos, + .nested_projected_fields = std::move(nested_projected_fields), + }); + } + return std::make_shared(std::move(projected_fields)); + } + + void Wrap(const StructLike& row) { + owned_row_.reset(); + row_ = &row; + } + + void Wrap(std::shared_ptr row) { + owned_row_ = std::move(row); + row_ = owned_row_.get(); + } + + Result GetField(size_t pos) const override { + ICEBERG_PRECHECK(row_ != nullptr, "ProjectedStructLike has no wrapped row"); + if (pos >= projected_fields_->size()) { + return InvalidArgument("Projected field index {} out of range (size: {})", pos, + projected_fields_->size()); + } + + const auto& projected_field = (*projected_fields_)[pos]; + ICEBERG_ASSIGN_OR_RAISE(auto scalar, + row_->GetField(projected_field.source_field_pos)); + if (projected_field.nested_projected_fields == nullptr || + std::holds_alternative(scalar)) { + return scalar; + } + + if (!std::holds_alternative>(scalar)) { + return InvalidSchema("Expected struct field id {} while projecting equality row", + projected_field.field_id); + } + + auto child = std::get>(std::move(scalar)); + if (child == nullptr) { + return Scalar{std::monostate{}}; + } + + auto projected_struct = nested_projected_structs_[pos]; + projected_struct->Wrap(std::move(child)); + return Scalar{std::static_pointer_cast(std::move(projected_struct))}; + } + + size_t num_fields() const override { return projected_fields_->size(); } + + private: + std::shared_ptr owned_row_; + const StructLike* row_ = nullptr; + std::shared_ptr projected_fields_; + std::vector> nested_projected_structs_; +}; + +Status ValidateEqualityIds(const DataFile& delete_file) { + if (delete_file.equality_ids.empty()) { + return InvalidArgument("Equality delete file '{}' has no equality field ids", + delete_file.file_path); + } + return {}; +} + +SchemaField WithType(const SchemaField& field, std::shared_ptr type) { + return SchemaField{field.field_id(), std::string(field.name()), std::move(type), + field.optional(), std::string(field.doc())}; +} + +std::shared_ptr SortStructFieldsById(const std::shared_ptr& type) { + if (type->type_id() != TypeId::kStruct) { + return type; + } + + const auto& struct_type = internal::checked_cast(*type); + auto source_fields = struct_type.fields(); + std::vector> sorted_types; + sorted_types.reserve(source_fields.size()); + bool changed = false; + for (const auto& field : source_fields) { + auto sorted_type = SortStructFieldsById(field.type()); + changed = changed || sorted_type != field.type(); + sorted_types.push_back(std::move(sorted_type)); + } + + const bool needs_sort = + !std::ranges::is_sorted(source_fields, {}, &SchemaField::field_id); + if (!changed && !needs_sort) { + return type; + } + + std::vector fields; + fields.reserve(source_fields.size()); + for (size_t pos = 0; pos < source_fields.size(); ++pos) { + const auto& field = source_fields[pos]; + fields.push_back(sorted_types[pos] == field.type() + ? field + : WithType(field, std::move(sorted_types[pos]))); + } + + if (needs_sort) { + std::ranges::sort(fields, {}, &SchemaField::field_id); + } + return std::make_shared(std::move(fields)); +} + +void SortFieldsById(std::vector& fields) { + for (auto& field : fields) { + auto sorted_type = SortStructFieldsById(field.type()); + if (sorted_type != field.type()) { + field = WithType(field, std::move(sorted_type)); + } + } + std::ranges::sort(fields, {}, &SchemaField::field_id); +} + +Result> ProjectFieldsById( + const Schema& schema, const std::set& selected_ids) { + std::unordered_set unordered_ids(selected_ids.begin(), selected_ids.end()); + ICEBERG_ASSIGN_OR_RAISE(auto projected_schema, schema.Project(unordered_ids)); + std::vector fields(projected_schema->fields().begin(), + projected_schema->fields().end()); + return fields; +} + +Result> ProjectEqualityKeyFields( + const Schema& schema, const std::set& selected_ids) { + ICEBERG_ASSIGN_OR_RAISE(auto fields, ProjectFieldsById(schema, selected_ids)); + // Equality-delete keys keep the projected struct shape; fields are sorted by id + // within each struct level, not flattened by nested leaf ids. + SortFieldsById(fields); + return fields; +} + +bool ContainsFieldId(const SchemaField& field, int32_t field_id); + +bool ContainsFieldId(const Type& type, int32_t field_id) { + if (!type.is_nested()) { + return false; + } + const auto& nested = internal::checked_cast(type); + return std::ranges::any_of(nested.fields(), [field_id](const SchemaField& field) { + return ContainsFieldId(field, field_id); + }); +} + +bool ContainsFieldId(const SchemaField& field, int32_t field_id) { + return field.field_id() == field_id || ContainsFieldId(*field.type(), field_id); +} + +Status ValidateEqualityProjectionField(int32_t field_id, const SchemaField& field) { + if (field.field_id() == field_id) { + if (!field.type()->is_primitive()) { + return InvalidArgument( + "Equality delete field id {} must reference a primitive field", field_id); + } + return {}; + } + + switch (field.type()->type_id()) { + case TypeId::kStruct: { + const auto& struct_type = internal::checked_cast(*field.type()); + for (const auto& child : struct_type.fields()) { + if (ContainsFieldId(child, field_id)) { + return ValidateEqualityProjectionField(field_id, child); + } + } + break; + } + case TypeId::kList: + case TypeId::kMap: + if (ContainsFieldId(*field.type(), field_id)) { + return InvalidArgument("Equality delete field id {} must not be nested in {}", + field_id, ToString(field.type()->type_id())); + } + break; + default: + break; + } + + return InvalidSchema("Cannot find equality delete field id {} in projection field {}", + field_id, field.field_id()); +} + +Result> LookupFieldInSchema( + const Schema& schema, int32_t field_id) { + ICEBERG_ASSIGN_OR_RAISE(auto field, schema.FindFieldById(field_id)); + if (!field.has_value()) { + return std::nullopt; + } + if (!field->get().type()->is_primitive()) { + return InvalidArgument("Equality delete field id {} must reference a primitive field", + field_id); + } + + std::set selected_ids = {field_id}; + ICEBERG_ASSIGN_OR_RAISE(auto projected_fields, ProjectFieldsById(schema, selected_ids)); + if (projected_fields.empty()) { + return InvalidSchema("Cannot project field id {} from lookup schema", field_id); + } + if (projected_fields.size() != 1) { + return InvalidSchema("Expected one top-level projection for field id {} but got {}", + field_id, projected_fields.size()); + } + + return DeleteFilter::FieldLookupResult{ + .field = field.value().get(), + .projection_field = std::move(projected_fields[0]), + }; +} + +Result MergeField(SchemaField& existing, const SchemaField& required) { + if (existing.field_id() != required.field_id()) { + return InvalidSchema("Cannot merge field id {} with field id {}", existing.field_id(), + required.field_id()); + } + + if (*existing.type() == *required.type() || !required.type()->is_nested()) { + return false; + } + + if (existing.type()->type_id() == TypeId::kStruct && + required.type()->type_id() == TypeId::kStruct) { + const auto& existing_struct = + internal::checked_cast(*existing.type()); + std::vector fields(existing_struct.fields().begin(), + existing_struct.fields().end()); + const auto& required_struct = + internal::checked_cast(*required.type()); + + bool changed = false; + for (const auto& required_child : required_struct.fields()) { + auto existing_pos = FindFieldIndexById(fields, required_child.field_id()); + if (existing_pos.has_value()) { + ICEBERG_ASSIGN_OR_RAISE(auto child_changed, + MergeField(fields[existing_pos.value()], required_child)); + changed = changed || child_changed; + } else { + fields.push_back(required_child); + changed = true; + } + } + + if (!changed) { + return false; + } + existing = SchemaField(existing.field_id(), std::string(existing.name()), + std::make_shared(std::move(fields)), + existing.optional(), std::string(existing.doc())); + return true; + } + + return InvalidArgument( + "Cannot merge non-struct nested field id {} into delete projection", + required.field_id()); +} + +Result MergeProjectionField(std::vector& fields, + const SchemaField& required_projection) { + auto existing_pos = FindFieldIndexById(fields, required_projection.field_id()); + if (existing_pos.has_value()) { + return MergeField(fields[existing_pos.value()], required_projection); + } + + fields.push_back(required_projection); + return true; +} + +void AddIdOnce(std::vector& ids, std::unordered_set& seen, + int32_t field_id) { + if (seen.insert(field_id).second) { + ids.push_back(field_id); + } +} + +} // namespace + +struct DeleteFilter::EqDeleteGroup { + std::unique_ptr row_projection; + std::unique_ptr delete_set; +}; + +Result DeleteFilter::MakeFieldLookup( + std::shared_ptr table_schema, + std::span> schemas) { + ICEBERG_PRECHECK(table_schema != nullptr, "Table schema must not be null"); + + std::vector> lookup_schemas; + lookup_schemas.reserve(schemas.size() + 1); + const int32_t current_schema_id = table_schema->schema_id(); + lookup_schemas.push_back(std::move(table_schema)); + + std::vector> sorted_fallback_schemas; + sorted_fallback_schemas.reserve(schemas.size()); + for (const auto& schema : schemas) { + ICEBERG_PRECHECK(schema != nullptr, "Schema must not be null"); + if (schema->schema_id() != current_schema_id) { + sorted_fallback_schemas.push_back(schema); + } + } + + // Search fallback schemas from latest to oldest so the highest schema_id wins. + std::ranges::stable_sort(sorted_fallback_schemas, [](const auto& lhs, const auto& rhs) { + return lhs->schema_id() > rhs->schema_id(); + }); + + std::unordered_set seen_schema_ids; + seen_schema_ids.insert(current_schema_id); + for (const auto& schema : sorted_fallback_schemas) { + if (seen_schema_ids.insert(schema->schema_id()).second) { + lookup_schemas.push_back(schema); + } + } + + return [lookup_schemas = std::move(lookup_schemas)]( + int32_t field_id) -> Result> { + for (const auto& schema : lookup_schemas) { + ICEBERG_ASSIGN_OR_RAISE(auto field, LookupFieldInSchema(*schema, field_id)); + if (field.has_value()) { + return field; + } + } + return std::nullopt; + }; +} + +Result DeleteFilter::MakeFieldLookup( + std::shared_ptr table_metadata) { + ICEBERG_PRECHECK(table_metadata != nullptr, "Table metadata must not be null"); + + ICEBERG_ASSIGN_OR_RAISE(auto table_schema, table_metadata->Schema()); + return MakeFieldLookup(std::move(table_schema), table_metadata->schemas); +} + +Result> DeleteFilter::Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_schema, std::shared_ptr requested_schema, + std::shared_ptr io, bool need_row_pos_col, + std::shared_ptr counter) { + ICEBERG_ASSIGN_OR_RAISE(auto field_lookup, MakeFieldLookup(table_schema)); + return Make(std::move(file_path), delete_files, std::move(requested_schema), + std::move(io), std::move(field_lookup), need_row_pos_col, + std::move(counter)); +} + +Result> DeleteFilter::Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_metadata, + std::shared_ptr requested_schema, std::shared_ptr io, + bool need_row_pos_col, std::shared_ptr counter) { + ICEBERG_PRECHECK(table_metadata != nullptr, "Table metadata must not be null"); + + ICEBERG_ASSIGN_OR_RAISE(auto field_lookup, MakeFieldLookup(std::move(table_metadata))); + return Make(std::move(file_path), delete_files, std::move(requested_schema), + std::move(io), std::move(field_lookup), need_row_pos_col, + std::move(counter)); +} + +Result> DeleteFilter::Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_schema, std::shared_ptr requested_schema, + std::shared_ptr io, std::span> schemas, + bool need_row_pos_col, std::shared_ptr counter) { + ICEBERG_ASSIGN_OR_RAISE(auto field_lookup, MakeFieldLookup(table_schema, schemas)); + return Make(std::move(file_path), delete_files, std::move(requested_schema), + std::move(io), std::move(field_lookup), need_row_pos_col, + std::move(counter)); +} + +Result> DeleteFilter::Make( + std::string file_path, std::span> delete_files, + std::shared_ptr requested_schema, std::shared_ptr io, + FieldLookup field_lookup, bool need_row_pos_col, + std::shared_ptr counter) { + ICEBERG_PRECHECK(requested_schema != nullptr, "Requested schema must not be null"); + ICEBERG_PRECHECK(field_lookup != nullptr, "Field lookup must not be null"); + ICEBERG_PRECHECK(delete_files.empty() || io != nullptr, + "FileIO must not be null when delete files are present"); + + auto filter = std::unique_ptr( + new DeleteFilter(std::move(file_path), std::move(requested_schema), std::move(io), + std::move(field_lookup), need_row_pos_col, std::move(counter))); + ICEBERG_RETURN_UNEXPECTED(filter->Init(delete_files)); + return filter; +} + +DeleteFilter::DeleteFilter(std::string file_path, + std::shared_ptr requested_schema, + std::shared_ptr io, FieldLookup field_lookup, + bool need_row_pos_col, std::shared_ptr counter) + : file_path_(std::move(file_path)), + requested_schema_(std::move(requested_schema)), + field_lookup_(std::move(field_lookup)), + need_row_pos_col_(need_row_pos_col), + counter_(std::move(counter)), + delete_loader_(std::move(io)) {} + +DeleteFilter::~DeleteFilter() = default; + +Status DeleteFilter::Init(std::span> delete_files) { + for (const auto& delete_file : delete_files) { + ICEBERG_PRECHECK(delete_file != nullptr, "Delete file must not be null"); + + switch (delete_file->content) { + case DataFile::Content::kPositionDeletes: + pos_deletes_.push_back(delete_file); + break; + case DataFile::Content::kEqualityDeletes: + ICEBERG_RETURN_UNEXPECTED(ValidateEqualityIds(*delete_file)); + eq_deletes_.push_back(delete_file); + break; + case DataFile::Content::kData: + return InvalidArgument("Expected delete file but got data file '{}'", + delete_file->file_path); + default: + return InvalidArgument("Unknown delete file content type {}", + static_cast(delete_file->content)); + } + } + + ICEBERG_ASSIGN_OR_RAISE(required_schema_, ComputeRequiredSchema()); + + // Pre-compute _pos column position for reuse + pos_field_position_ = FindFieldIndexById(required_schema_->fields(), + MetadataColumns::kFilePositionColumnId); + + return {}; +} + +Result> DeleteFilter::ComputeRequiredSchema() const { + if (!HasPositionDeletes() && !HasEqualityDeletes()) { + return requested_schema_; + } + + std::vector required_ids; + std::unordered_set seen_required_ids; + if (HasPositionDeletes() && need_row_pos_col_) { + AddIdOnce(required_ids, seen_required_ids, MetadataColumns::kFilePositionColumnId); + } + + for (const auto& delete_file : eq_deletes_) { + for (int32_t field_id : delete_file->equality_ids) { + AddIdOnce(required_ids, seen_required_ids, field_id); + } + } + + std::vector fields(requested_schema_->fields().begin(), + requested_schema_->fields().end()); + bool changed = false; + + for (int32_t field_id : required_ids) { + if (field_id == MetadataColumns::kFilePositionColumnId || + field_id == MetadataColumns::kIsDeletedColumnId) { + // These columns do not exist in the table schema and will be handled later. + continue; + } + + // Top-level primitive fields already cover equality-delete needs. Nested fields + // still need lookup so we can validate/merge the required subfield projection. + auto existing_pos = FindFieldIndexById(fields, field_id); + if (existing_pos.has_value() && !fields[existing_pos.value()].type()->is_nested()) { + continue; + } + + ICEBERG_ASSIGN_OR_RAISE(auto lookup, field_lookup_(field_id)); + if (!lookup.has_value()) { + return InvalidArgument("Cannot find equality delete field id {}", field_id); + } + ICEBERG_RETURN_UNEXPECTED( + ValidateEqualityProjectionField(field_id, lookup->projection_field)); + + ICEBERG_ASSIGN_OR_RAISE(auto merged, + MergeProjectionField(fields, lookup->projection_field)); + changed = changed || merged; + } + + const bool needs_pos = + HasPositionDeletes() && need_row_pos_col_ && + !FindFieldIndexById(fields, MetadataColumns::kFilePositionColumnId).has_value(); + if (needs_pos) { + fields.push_back(MetadataColumns::kRowPosition); + changed = true; + } + + if (!changed) { + return requested_schema_; + } + + return std::make_shared(std::move(fields)); +} + +const std::shared_ptr& DeleteFilter::RequiredSchema() const { + return required_schema_; +} + +bool DeleteFilter::HasPositionDeletes() const { return !pos_deletes_.empty(); } + +bool DeleteFilter::HasEqualityDeletes() const { return !eq_deletes_.empty(); } + +Status DeleteFilter::EnsurePositionDeletesLoaded() const { + if (!HasPositionDeletes()) { + return {}; + } + + std::lock_guard lock(pos_mutex_); + if (pos_loaded_) { + return {}; + } + + ICEBERG_ASSIGN_OR_RAISE(pos_index_, + delete_loader_.LoadPositionDeletes(pos_deletes_, file_path_)); + pos_loaded_ = true; + return {}; +} + +Status DeleteFilter::EnsureEqualityDeletesLoaded() const { + if (!HasEqualityDeletes()) { + return {}; + } + + std::lock_guard lock(eq_mutex_); + if (eq_loaded_) { + return {}; + } + + std::map, std::vector>> files_by_ids; + for (const auto& delete_file : eq_deletes_) { + // equality_ids were already validated in Init, build the grouping key directly. + std::set ids(delete_file->equality_ids.begin(), + delete_file->equality_ids.end()); + files_by_ids[std::move(ids)].push_back(delete_file); + } + + std::vector> groups; + groups.reserve(files_by_ids.size()); + + for (auto& [field_ids, files] : files_by_ids) { + ICEBERG_ASSIGN_OR_RAISE(auto fields, + ProjectEqualityKeyFields(*required_schema_, field_ids)); + auto equality_type = std::make_shared(std::move(fields)); + + ICEBERG_ASSIGN_OR_RAISE(auto row_projection, ProjectedStructLike::BuildProjection( + *required_schema_, *equality_type)); + auto project_row = std::make_unique(std::move(row_projection)); + ICEBERG_ASSIGN_OR_RAISE(auto delete_set, + delete_loader_.LoadEqualityDeletes(files, *equality_type)); + groups.push_back(std::make_unique(EqDeleteGroup{ + .row_projection = std::move(project_row), + .delete_set = std::move(delete_set), + })); + } + + eq_groups_ = std::move(groups); + eq_loaded_ = true; + return {}; +} + +const std::shared_ptr& DeleteFilter::ExpectedSchema() const { + return requested_schema_; +} + +void DeleteFilter::IncrementDeleteCount(int64_t count) { + if (counter_ != nullptr) { + counter_->Increment(count); + } +} + +Result DeleteFilter::DeletedRowPositions() const { + if (!HasPositionDeletes()) { + return nullptr; + } + ICEBERG_RETURN_UNEXPECTED(EnsurePositionDeletesLoaded()); + return &pos_index_; +} + +Result(const StructLike&)>> DeleteFilter::EqDeletedRowFilter() + const { + if (!HasEqualityDeletes()) { + // No equality deletes: every row is alive. + return [](const StructLike&) -> Result { return true; }; + } + ICEBERG_RETURN_UNEXPECTED(EnsureEqualityDeletesLoaded()); + std::lock_guard lock(eq_mutex_); + if (!eq_deleted_row_filter_cache_) { + eq_deleted_row_filter_cache_ = [this](const StructLike& row) -> Result { + for (const auto& group : eq_groups_) { + auto& projected_row = *group->row_projection; + projected_row.Wrap(row); + ICEBERG_ASSIGN_OR_RAISE(auto matched, group->delete_set->Contains(projected_row)); + if (matched) { + return false; + } + } + return true; + }; + } + return eq_deleted_row_filter_cache_; +} + +Result(const StructLike&)>> +DeleteFilter::FindEqualityDeleteRows() const { + if (!HasEqualityDeletes()) { + // No equality deletes: no row is deleted. + return [](const StructLike&) -> Result { return false; }; + } + ICEBERG_ASSIGN_OR_RAISE(auto alive_filter, EqDeletedRowFilter()); + return [alive_filter = std::move(alive_filter)](const StructLike& row) -> Result { + ICEBERG_ASSIGN_OR_RAISE(auto alive, alive_filter(row)); + return !alive; + }; +} + +Result DeleteFilter::ComputeAliveRows(const ArrowSchema& batch_schema, + const ArrowArray& batch) const { + ICEBERG_PRECHECK(batch.length >= 0, "Batch length must be non-negative"); + + ICEBERG_RETURN_UNEXPECTED(EnsurePositionDeletesLoaded()); + ICEBERG_RETURN_UNEXPECTED(EnsureEqualityDeletesLoaded()); + + AliveRowSelection result; + if (batch.length == 0) { + return result; + } + + result.indices.reserve(batch.length); + ICEBERG_ASSIGN_OR_RAISE(auto row, ArrowArrayStructLike::Make(batch_schema, batch)); + + for (int64_t i = 0; i < batch.length; ++i) { + if (i > 0) { + ICEBERG_RETURN_UNEXPECTED(row->Reset(i)); + } + + bool deleted = false; + if (pos_field_position_.has_value()) { + ICEBERG_ASSIGN_OR_RAISE(auto pos_scalar, + row->GetField(pos_field_position_.value())); + auto* pos = std::get_if(&pos_scalar); + if (pos == nullptr) { + return InvalidArrowData("Position delete filtering requires non-null int64 _pos"); + } + deleted = pos_index_.IsDeleted(*pos); + } + + if (!deleted) { + for (const auto& eq_group : eq_groups_) { + auto& projected_row = *eq_group->row_projection; + projected_row.Wrap(*row); + ICEBERG_ASSIGN_OR_RAISE(auto matched, + eq_group->delete_set->Contains(projected_row)); + if (matched) { + deleted = true; + break; + } + } + } + + if (!deleted) { + result.indices.push_back(static_cast(i)); + } else if (counter_ != nullptr) { + counter_->Increment(); + } + } + + return result; +} + +} // namespace iceberg diff --git a/src/iceberg/data/delete_filter.h b/src/iceberg/data/delete_filter.h new file mode 100644 index 000000000..4cb9bace6 --- /dev/null +++ b/src/iceberg/data/delete_filter.h @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/data/delete_filter.h +/// Delete-aware filtering for Arrow C Data batches. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/arrow_c_data.h" +#include "iceberg/data/delete_loader.h" +#include "iceberg/deletes/position_delete_index.h" +#include "iceberg/iceberg_data_export.h" +#include "iceberg/result.h" +#include "iceberg/schema_field.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Result of ComputeAliveRows: indices of rows not matched by any delete. +struct ICEBERG_DATA_EXPORT AliveRowSelection { + /// Zero-based row indices within the batch that are alive (not deleted). + std::vector indices; + + /// Number of alive rows (convenience accessor to avoid size_t casts). + int64_t alive_count() const { return static_cast(indices.size()); } + + bool empty() const { return indices.empty(); } +}; + +/// \brief Counts rows removed by delete filters. +class ICEBERG_DATA_EXPORT DeleteCounter { + public: + void Increment(int64_t count = 1) { + count_.fetch_add(count, std::memory_order_relaxed); + } + int64_t Get() const { return count_.load(std::memory_order_relaxed); } + + private: + std::atomic count_{0}; +}; + +/// \brief Concrete batch-oriented delete filter for merge-on-read data batches. +class ICEBERG_DATA_EXPORT DeleteFilter { + public: + /// \brief Field lookup output for current or fallback equality-delete fields. + /// + /// `field` is the exact field for validation. `projection_field` is the + /// top-level field, possibly with a pruned nested struct path, that must be + /// merged into RequiredSchema so the data reader can materialize the delete + /// column. + struct FieldLookupResult { + SchemaField field; + SchemaField projection_field; + }; + + /// \brief Lookup a field by ID, including fields from table schema fallbacks. + using FieldLookup = std::function>(int32_t)>; + + /// \brief Build a lookup from the current schema and optional table schemas. + /// + /// The current table schema is searched first. `schemas` is the table metadata + /// schema list and may contain `table_schema`; current schema duplicates are ignored + /// and fallback schemas are searched from latest schema id to oldest. + static Result MakeFieldLookup( + std::shared_ptr table_schema, + std::span> schemas = {}); + + /// \brief Build a lookup from table metadata which uses the current schema first, + /// then table metadata schemas as fallback. + static Result MakeFieldLookup( + std::shared_ptr table_metadata); + + /// \brief Create a DeleteFilter with current schema only field lookup. + /// + /// \param need_row_pos_col If true, `_pos` is added to `RequiredSchema` when + /// position deletes are present so `ComputeAliveRows` can apply them. + /// Pass false when the caller owns position filtering externally (e.g. a vectorised + /// reader that applies the position delete index directly to Arrow column buffers). + /// Note that when `need_row_pos_col` is false, `HasPositionDeletes()` may + /// return true but `ComputeAliveRows` will not apply position deletes because `_pos` + /// is absent from `RequiredSchema`. The caller is responsible for applying them. + /// \param counter Optional counter incremented for each deleted row. + static Result> Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_schema, std::shared_ptr requested_schema, + std::shared_ptr io, bool need_row_pos_col = true, + std::shared_ptr counter = nullptr); + + /// \brief Create a DeleteFilter using table metadata for schema-aware field lookup. + static Result> Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_metadata, + std::shared_ptr requested_schema, std::shared_ptr io, + bool need_row_pos_col = true, std::shared_ptr counter = nullptr); + + /// \brief Create a DeleteFilter with table schemas for dropped equality fields. + static Result> Make( + std::string file_path, std::span> delete_files, + std::shared_ptr table_schema, std::shared_ptr requested_schema, + std::shared_ptr io, std::span> schemas, + bool need_row_pos_col = true, std::shared_ptr counter = nullptr); + + /// \brief Create a DeleteFilter with a custom field lookup. + static Result> Make( + std::string file_path, std::span> delete_files, + std::shared_ptr requested_schema, std::shared_ptr io, + FieldLookup field_lookup, bool need_row_pos_col = true, + std::shared_ptr counter = nullptr); + + ~DeleteFilter(); + + /// \brief Schema required from the underlying data file reader. + const std::shared_ptr& RequiredSchema() const; + + /// \brief The original schema requested by the caller, before delete columns were + /// added. + const std::shared_ptr& ExpectedSchema() const; + + /// \brief Increment the delete counter by the given count. + /// + /// Allows callers to record deletes that occur outside `ComputeAliveRows` (e.g. when + /// applying deletes in a vectorised path). + void IncrementDeleteCount(int64_t count = 1); + + /// \brief Expose the loaded position delete index for external use. + /// + /// Triggers lazy loading of position delete files on first call. Returns nullptr + /// when there are no position deletes. Returns an error if loading fails. + /// + /// The returned pointer is valid only for the lifetime of this DeleteFilter. + Result DeletedRowPositions() const; + + /// \brief Returns a predicate that is true for rows NOT matched by any equality delete. + /// + /// The returned function is valid for the lifetime of this DeleteFilter and is cached + /// after the first call. When there are no equality deletes, returns a predicate that + /// always returns true (every row is alive). + /// + /// \note The returned predicate is NOT thread-safe: it mutates internal projection + /// state on each call. Do not invoke it concurrently from multiple threads. + Result(const StructLike&)>> EqDeletedRowFilter() const; + + /// \brief Returns a predicate that is true for rows matched by any equality delete. + /// + /// Inverse of `EqDeletedRowFilter()`. When there are no equality deletes, returns a + /// predicate that always returns false (no row is deleted). + Result(const StructLike&)>> FindEqualityDeleteRows() const; + + /// \brief Compute alive rows relative to the supplied Arrow C Data batch. + /// + /// Returns the indices (zero-based, relative to the batch) of rows not matched by + /// any delete. Deleted-row counts are forwarded to the DeleteCounter supplied at + /// construction. + Result ComputeAliveRows(const ArrowSchema& batch_schema, + const ArrowArray& batch) const; + + bool HasPositionDeletes() const; + bool HasEqualityDeletes() const; + + DeleteFilter(const DeleteFilter&) = delete; + DeleteFilter& operator=(const DeleteFilter&) = delete; + + private: + struct EqDeleteGroup; + + DeleteFilter(std::string file_path, std::shared_ptr requested_schema, + std::shared_ptr io, FieldLookup field_lookup, + bool need_row_pos_col, std::shared_ptr counter); + + Status Init(std::span> delete_files); + Result> ComputeRequiredSchema() const; + Status EnsurePositionDeletesLoaded() const; + Status EnsureEqualityDeletesLoaded() const; + + const std::string file_path_; + std::vector> pos_deletes_; + std::vector> eq_deletes_; + + std::shared_ptr requested_schema_; + std::shared_ptr required_schema_; + FieldLookup field_lookup_; + + const bool need_row_pos_col_; + // Position of `_pos` in required_schema_ when existent + std::optional pos_field_position_; + std::shared_ptr counter_; + + // TODO(gangwu): expose a factory hook (e.g. a std::function or a + // virtual newDeleteLoader()) so callers can inject a caching DeleteLoader (analogous to + // SparkDeleteFilter.CachingDeleteLoader in Java). + DeleteLoader delete_loader_; + + mutable std::mutex pos_mutex_; + mutable bool pos_loaded_ = false; + mutable PositionDeleteIndex pos_index_; + + mutable std::mutex eq_mutex_; + mutable bool eq_loaded_ = false; + mutable std::vector> eq_groups_; + mutable std::function(const StructLike&)> eq_deleted_row_filter_cache_; +}; + +} // namespace iceberg diff --git a/src/iceberg/data/meson.build b/src/iceberg/data/meson.build index 0f68deccf..f0877ec64 100644 --- a/src/iceberg/data/meson.build +++ b/src/iceberg/data/meson.build @@ -18,6 +18,7 @@ install_headers( [ 'data_writer.h', + 'delete_filter.h', 'delete_loader.h', 'equality_delete_writer.h', 'position_delete_writer.h', diff --git a/src/iceberg/file_io.cc b/src/iceberg/file_io.cc index d76ffeb60..e4223182e 100644 --- a/src/iceberg/file_io.cc +++ b/src/iceberg/file_io.cc @@ -100,4 +100,11 @@ Status FileIO::WriteFile(const std::string& file_location, std::string_view cont return FinishWithCloseStatus(std::move(status), stream->Close()); } +Status FileIO::DeleteFiles(const std::vector& file_locations) { + for (const auto& file_location : file_locations) { + ICEBERG_RETURN_UNEXPECTED(DeleteFile(file_location)); + } + return {}; +} + } // namespace iceberg diff --git a/src/iceberg/file_io.h b/src/iceberg/file_io.h index e772b5336..1f91fb0c1 100644 --- a/src/iceberg/file_io.h +++ b/src/iceberg/file_io.h @@ -26,6 +26,7 @@ #include #include #include +#include #include "iceberg/iceberg_export.h" #include "iceberg/result.h" @@ -154,6 +155,16 @@ class ICEBERG_EXPORT FileIO { virtual Status DeleteFile(const std::string& file_location) { return NotImplemented("DeleteFile not implemented"); } + + /// \brief Delete files at the given locations. + /// + /// Implementations that can delete multiple files efficiently should override this + /// method. The default implementation deletes files sequentially using DeleteFile + /// and returns the first error encountered. + /// + /// \param file_locations The locations of the files to delete. + /// \return void if all deletes succeed, or an error code if any delete fails. + virtual Status DeleteFiles(const std::vector& file_locations); }; } // namespace iceberg diff --git a/src/iceberg/manifest/manifest_filter_manager.cc b/src/iceberg/manifest/manifest_filter_manager.cc new file mode 100644 index 000000000..d25fe2d0c --- /dev/null +++ b/src/iceberg/manifest/manifest_filter_manager.cc @@ -0,0 +1,456 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/manifest/manifest_filter_manager.h" + +#include +#include +#include + +#include "iceberg/expression/expression.h" +#include "iceberg/expression/expressions.h" +#include "iceberg/expression/inclusive_metrics_evaluator.h" +#include "iceberg/expression/manifest_evaluator.h" +#include "iceberg/expression/residual_evaluator.h" +#include "iceberg/expression/strict_metrics_evaluator.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/partition_spec.h" +#include "iceberg/result.h" +#include "iceberg/snapshot.h" +#include "iceberg/table_metadata.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { + +using PartitionSpecsById = ManifestFilterManager::PartitionSpecsById; + +bool HasRowFilterExpression(const std::shared_ptr& expr) { + return expr != nullptr && expr->op() != Expression::Operation::kFalse; +} + +Result> PartitionSpecById( + const PartitionSpecsById& specs_by_id, int32_t spec_id) { + auto iter = specs_by_id.find(spec_id); + if (iter == specs_by_id.end() || iter->second == nullptr) { + return NotFound("Partition spec with ID {} is not found", spec_id); + } + return iter->second; +} + +Result FormatPartitionPath(const PartitionSpecsById& specs_by_id, + const DataFile& file, int32_t spec_id) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, PartitionSpecById(specs_by_id, spec_id)); + return spec->PartitionPath(file.partition); +} + +} // namespace + +ManifestFilterManager::ManifestFilterManager(ManifestContent content, + std::shared_ptr file_io) + : manifest_content_(content), + file_io_(std::move(file_io)), + delete_expr_(Expressions::AlwaysFalse()) {} + +ManifestFilterManager::~ManifestFilterManager() = default; + +Status ManifestFilterManager::DeleteByRowFilter(std::shared_ptr expr) { + ICEBERG_PRECHECK(expr != nullptr, "Cannot delete files using filter: null"); + ICEBERG_ASSIGN_OR_RAISE(delete_expr_, Or::MakeFolded(delete_expr_, std::move(expr))); + manifest_evaluator_cache_.clear(); + residual_evaluator_cache_.clear(); + return {}; +} + +void ManifestFilterManager::CaseSensitive(bool case_sensitive) { + case_sensitive_ = case_sensitive; + manifest_evaluator_cache_.clear(); + residual_evaluator_cache_.clear(); +} + +void ManifestFilterManager::DeleteFile(std::string_view path) { + delete_paths_.insert(std::string(path)); +} + +Status ManifestFilterManager::DeleteFile(std::shared_ptr file) { + ICEBERG_PRECHECK(file != nullptr, "Cannot delete file: null"); + delete_paths_.insert(file->file_path); + delete_files_.insert(std::move(file)); + return {}; +} + +const DataFileSet& ManifestFilterManager::FilesToBeDeleted() const { + return delete_files_; +} + +void ManifestFilterManager::DropPartition(int32_t spec_id, PartitionValues partition) { + drop_partitions_.add(spec_id, std::move(partition)); +} + +void ManifestFilterManager::FailMissingDeletePaths() { + fail_missing_delete_paths_ = true; +} + +void ManifestFilterManager::FailAnyDelete() { fail_any_delete_ = true; } + +bool ManifestFilterManager::ContainsDeletes() const { + return HasRowFilterExpression(delete_expr_) || !delete_paths_.empty() || + !drop_partitions_.empty(); +} + +void ManifestFilterManager::DropDeleteFilesOlderThan(int64_t sequence_number) { + min_sequence_number_ = sequence_number; +} + +void ManifestFilterManager::RemoveDanglingDeletesFor(const DataFileSet& deleted_files) { + for (const auto& file : deleted_files) { + removed_data_file_paths_.insert(file->file_path); + } +} + +Result ManifestFilterManager::CanContainDroppedFiles(const ManifestFile&) const { + // TODO(Guotao): Use the manifest descriptor to skip unrelated object-delete + // manifests once object-delete partitions are tracked separately. + // Currently, DeleteFile(std::shared_ptr) degrades to a path-based delete, + // which forces scanning all manifests. + // Also open delete manifests when a minimum sequence number is set for cleanup. + return !delete_paths_.empty() || !removed_data_file_paths_.empty() || + (manifest_content_ == ManifestContent::kDeletes && min_sequence_number_ > 0); +} + +Result ManifestFilterManager::CanContainDroppedPartitions( + const ManifestFile& manifest) const { + if (drop_partitions_.empty()) return false; + // TODO(Guotao): Use partition_summaries bounds to skip manifests that cannot + // contain any dropped partition, instead of only matching partition spec IDs. + // Only manifests whose partition spec matches a registered drop can contain + // entries for that partition. PartitionKey is pair. + int32_t spec_id = manifest.partition_spec_id; + for (const auto& key : drop_partitions_) { + if (key.first == spec_id) return true; + } + return false; +} + +Result ManifestFilterManager::CanContainExpressionDeletes( + const ManifestFile& manifest, const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id) { + if (!HasRowFilterExpression(delete_expr_)) return false; + int32_t spec_id = manifest.partition_spec_id; + ICEBERG_ASSIGN_OR_RAISE(auto* evaluator, + GetManifestEvaluator(schema, specs_by_id, spec_id)); + return evaluator->Evaluate(manifest); +} + +Result ManifestFilterManager::CanContainDeletedFiles( + const ManifestFile& manifest, const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, bool trust_manifest_references) { + // A manifest with no live files cannot contain files to delete. + // Missing counts mean the count is unknown; treat it as possibly non-zero. + bool has_live = !manifest.added_files_count.has_value() || + manifest.added_files_count.value() > 0 || + !manifest.existing_files_count.has_value() || + manifest.existing_files_count.value() > 0; + if (!has_live) return false; + + if (trust_manifest_references) { + // TODO(Guotao): Return whether this manifest is in the referenced manifest set. + return true; + } + + ICEBERG_ASSIGN_OR_RAISE(auto can_contain_dropped_files, + CanContainDroppedFiles(manifest)); + if (can_contain_dropped_files) return true; + + ICEBERG_ASSIGN_OR_RAISE(auto can_contain_expression_deletes, + CanContainExpressionDeletes(manifest, schema, specs_by_id)); + if (can_contain_expression_deletes) return true; + + return CanContainDroppedPartitions(manifest); +} + +Result ManifestFilterManager::GetManifestEvaluator( + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + int32_t spec_id) { + auto& evaluator = manifest_evaluator_cache_[spec_id]; + if (!evaluator) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, PartitionSpecById(specs_by_id, spec_id)); + ICEBERG_ASSIGN_OR_RAISE(evaluator, ManifestEvaluator::MakeRowFilter( + delete_expr_, spec, *schema, case_sensitive_)); + } + return evaluator.get(); +} + +Result ManifestFilterManager::GetResidualEvaluator( + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + int32_t spec_id) { + auto& evaluator = residual_evaluator_cache_[spec_id]; + if (!evaluator) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, PartitionSpecById(specs_by_id, spec_id)); + ICEBERG_ASSIGN_OR_RAISE(evaluator, ResidualEvaluator::Make(delete_expr_, *spec, + *schema, case_sensitive_)); + } + return evaluator.get(); +} + +Result ManifestFilterManager::ShouldDelete(const ManifestEntry& entry, + const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + int32_t manifest_spec_id) { + if (!entry.data_file) return false; + const DataFile& file = *entry.data_file; + int32_t spec_id = file.partition_spec_id.value_or(manifest_spec_id); + + // Path-based and partition-drop checks + if (delete_paths_.count(file.file_path) || + drop_partitions_.contains(spec_id, file.partition)) { + if (fail_any_delete_) { + ICEBERG_ASSIGN_OR_RAISE(auto partition_path, + FormatPartitionPath(specs_by_id, file, spec_id)); + return InvalidArgument("Operation would delete existing data: {}", partition_path); + } + return true; + } + + // Delete-manifest-specific cleanup (only for ManifestContent::kDeletes). + if (manifest_content_ == ManifestContent::kDeletes) { + // Drop delete files whose data sequence number is older than the minimum + // retained by the table (they can no longer match any live data rows). + int64_t seq = entry.sequence_number.value_or(0); + if (min_sequence_number_ > 0 && seq > 0 && seq < min_sequence_number_) { + return true; + } + + // Drop DVs that reference a data file that has been removed (dangling DV). + if (!removed_data_file_paths_.empty() && file.IsDeletionVector() && + file.referenced_data_file.has_value() && + removed_data_file_paths_.count(*file.referenced_data_file)) { + return true; + } + } + + if (HasRowFilterExpression(delete_expr_)) { + ICEBERG_ASSIGN_OR_RAISE(auto* residual_eval, + GetResidualEvaluator(schema, specs_by_id, spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto residual_expr, + residual_eval->ResidualFor(file.partition)); + // TODO(Guotao): Cache strict/inclusive metrics evaluators per partition residual. + ICEBERG_ASSIGN_OR_RAISE( + auto strict_eval, + StrictMetricsEvaluator::Make(residual_expr, schema, case_sensitive_)); + ICEBERG_ASSIGN_OR_RAISE(auto strict_match, strict_eval->Evaluate(file)); + if (strict_match) { + if (fail_any_delete_) { + ICEBERG_ASSIGN_OR_RAISE(auto partition_path, + FormatPartitionPath(specs_by_id, file, spec_id)); + return InvalidArgument("Operation would delete existing data: {}", + partition_path); + } + return true; + } + + ICEBERG_ASSIGN_OR_RAISE(auto incl_eval, InclusiveMetricsEvaluator::Make( + residual_expr, *schema, case_sensitive_)); + ICEBERG_ASSIGN_OR_RAISE(auto incl_match, incl_eval->Evaluate(file)); + if (incl_match) { + if (manifest_content_ == ManifestContent::kDeletes) { + return false; + } + return InvalidArgument( + "Cannot delete file where some, but not all, rows match filter: {}", + file.file_path); + } + } + + return false; +} + +bool ManifestFilterManager::CanTrustManifestReferences( + const std::vector&) const { + // TODO(Guotao): Track source manifest locations for object deletes so manifests + // outside the referenced set can be skipped before any other delete checks. + return false; +} + +Result ManifestFilterManager::FilterManifest( + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + const ManifestFile& manifest, bool trust_manifest_references, + const ManifestWriterFactory& writer_factory, + std::unordered_set& found_paths) { + ICEBERG_ASSIGN_OR_RAISE( + auto can_contain_deleted_files, + CanContainDeletedFiles(manifest, schema, specs_by_id, trust_manifest_references)); + if (!can_contain_deleted_files) { + return manifest; + } + + int32_t spec_id = manifest.partition_spec_id; + ICEBERG_ASSIGN_OR_RAISE(auto spec, PartitionSpecById(specs_by_id, spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(manifest, file_io_, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->LiveEntries()); + + ICEBERG_ASSIGN_OR_RAISE(auto has_deleted_files, + ManifestHasDeletedFiles(entries, schema, specs_by_id, spec_id)); + if (!has_deleted_files) { + return manifest; + } + + return FilterManifestWithDeletedFiles(entries, spec_id, schema, specs_by_id, + writer_factory, found_paths); +} + +Result ManifestFilterManager::ManifestHasDeletedFiles( + const std::vector& entries, const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, int32_t manifest_spec_id) { + for (const auto& entry : entries) { + ICEBERG_ASSIGN_OR_RAISE(auto should_delete, + ShouldDelete(entry, schema, specs_by_id, manifest_spec_id)); + if (should_delete) { + return true; + } + } + return false; +} + +Result ManifestFilterManager::FilterManifestWithDeletedFiles( + const std::vector& entries, int32_t manifest_spec_id, + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + const ManifestWriterFactory& writer_factory, + std::unordered_set& found_paths) { + ICEBERG_ASSIGN_OR_RAISE(auto writer, + writer_factory(manifest_spec_id, manifest_content_)); + for (const auto& entry : entries) { + ICEBERG_ASSIGN_OR_RAISE(auto should_delete, + ShouldDelete(entry, schema, specs_by_id, manifest_spec_id)); + if (should_delete) { + if (entry.data_file && delete_paths_.count(entry.data_file->file_path)) { + found_paths.insert(entry.data_file->file_path); + } + if (entry.data_file) { + // TODO(Guotao): Track duplicate deletes and avoid full DataFile copies when + // summary generation can use lighter records. + delete_files_.insert(std::make_shared(*entry.data_file)); + } + ICEBERG_RETURN_UNEXPECTED(writer->WriteDeletedEntry(entry)); + } else { + ICEBERG_RETURN_UNEXPECTED(writer->WriteExistingEntry(entry)); + } + } + + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + return writer->ToManifestFile(); +} + +Status ManifestFilterManager::ValidateRequiredDeletes( + const std::unordered_set& found_paths) const { + if (!fail_missing_delete_paths_) { + return {}; + } + + std::string missing; + for (const auto& path : delete_paths_) { + if (!found_paths.count(path)) { + if (!missing.empty()) missing += ", "; + missing += path; + } + } + if (!missing.empty()) { + return InvalidArgument("Missing delete paths: {}", missing); + } + return {}; +} + +Result> ManifestFilterManager::FilterManifests( + const TableMetadata& metadata, const std::shared_ptr& base_snapshot, + const ManifestWriterFactory& writer_factory) { + if (!base_snapshot) { + ICEBERG_RETURN_UNEXPECTED(ValidateRequiredDeletes({})); + return std::vector{}; + } + + ICEBERG_PRECHECK(file_io_ != nullptr, "Cannot filter manifests: FileIO is null"); + + ICEBERG_ASSIGN_OR_RAISE( + auto list_reader, ManifestListReader::Make(base_snapshot->manifest_list, file_io_)); + ICEBERG_ASSIGN_OR_RAISE(auto all_manifests, list_reader->Files()); + + std::vector manifests; + manifests.reserve(all_manifests.size()); + for (auto& manifest : all_manifests) { + manifests.push_back(&manifest); + } + + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + TableMetadataCache metadata_cache(&metadata); + ICEBERG_ASSIGN_OR_RAISE(auto specs_by_id, metadata_cache.GetPartitionSpecsById()); + + return FilterManifests(schema, specs_by_id.get(), manifests, writer_factory); +} + +Result> ManifestFilterManager::FilterManifests( + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + const std::vector& input_manifests, + const ManifestWriterFactory& writer_factory) { + ICEBERG_PRECHECK(schema != nullptr, "Cannot filter manifests: schema is null"); + ICEBERG_PRECHECK(file_io_ != nullptr, "Cannot filter manifests: FileIO is null"); + + std::vector manifests; + manifests.reserve(input_manifests.size()); + for (const auto* manifest : input_manifests) { + ICEBERG_PRECHECK(manifest != nullptr, "Cannot filter manifests: manifest is null"); + if (manifest->content == manifest_content_) { + manifests.push_back(manifest); + } + } + + std::unordered_set found_paths; + if (manifests.empty()) { + ICEBERG_RETURN_UNEXPECTED(ValidateRequiredDeletes(found_paths)); + return std::vector{}; + } + + bool trust_manifest_references = CanTrustManifestReferences(manifests); + manifest_evaluator_cache_.clear(); + residual_evaluator_cache_.clear(); + replaced_manifests_count_ = 0; + + // TODO(Guotao): Parallelize manifest filtering with per-manifest results, then + // merge found paths and deleted files after the loop. + std::vector filtered; + filtered.reserve(manifests.size()); + for (const auto* manifest_ptr : manifests) { + ICEBERG_ASSIGN_OR_RAISE( + auto filtered_manifest, + FilterManifest(schema, specs_by_id, *manifest_ptr, trust_manifest_references, + writer_factory, found_paths)); + if (filtered_manifest.manifest_path != manifest_ptr->manifest_path) { + ++replaced_manifests_count_; + } + filtered.push_back(std::move(filtered_manifest)); + } + + ICEBERG_RETURN_UNEXPECTED(ValidateRequiredDeletes(found_paths)); + return filtered; +} + +} // namespace iceberg diff --git a/src/iceberg/manifest/manifest_filter_manager.h b/src/iceberg/manifest/manifest_filter_manager.h new file mode 100644 index 000000000..981b9ac3b --- /dev/null +++ b/src/iceberg/manifest/manifest_filter_manager.h @@ -0,0 +1,263 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/manifest/manifest_filter_manager.h +/// Filters an existing snapshot's manifest list, marking data files as DELETED +/// or EXISTING based on row-filter expressions, exact path deletes, and partition drops. + +#include +#include +#include +#include +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" +#include "iceberg/util/data_file_set.h" +#include "iceberg/util/partition_value_util.h" + +namespace iceberg { + +/// \brief Filters an existing snapshot's manifest list. +/// +/// The manager accumulates delete conditions incrementally, then applies them all +/// at once in a single FilterManifests() call. Manifests that contain no deleted +/// entries are returned unchanged (no I/O). Manifests that do contain deleted +/// entries are rewritten with those entries marked DELETED. +/// +/// The manager is content-agnostic: pass ManifestContent::kData to process data +/// manifests, or ManifestContent::kDeletes to process delete manifests. +/// +/// TODO(Guotao): For ManifestContent::kDeletes, implement cleanup for orphan delete files +/// and dangling deletion vectors. +/// +/// \note This class is non-copyable and non-movable. +class ICEBERG_EXPORT ManifestFilterManager { + public: + using PartitionSpecsById = std::unordered_map>; + + ManifestFilterManager(ManifestContent content, std::shared_ptr file_io); + ~ManifestFilterManager(); + + ManifestFilterManager(const ManifestFilterManager&) = delete; + ManifestFilterManager& operator=(const ManifestFilterManager&) = delete; + + /// \brief Register a row-filter expression. + /// + /// Any manifest entry whose column metrics indicate the file may satisfy the + /// expression will be marked DELETED. + /// + /// \param expr The expression to match files against + Status DeleteByRowFilter(std::shared_ptr expr); + + /// \brief Set whether row-filter field binding is case-sensitive. + void CaseSensitive(bool case_sensitive); + + /// \brief Register an exact file path for deletion. + /// + /// Any manifest entry whose file_path matches this path will be marked DELETED. + /// + /// \param path The exact file path to delete + void DeleteFile(std::string_view path); + + /// \brief Register a file object for deletion. + /// + /// Any manifest entry whose file_path matches file->file_path will be marked + /// DELETED. The file object is retained in FilesToBeDeleted(), allowing callers + /// to enumerate deleted file objects for follow-up delete-file cleanup. + /// Duplicate registrations (same path) are silently ignored. + /// + /// \param file The data/delete file to delete (must not be null) + Status DeleteFile(std::shared_ptr file); + + /// \brief Returns the set of file objects marked for deletion by this manager. + /// + /// This includes files registered via DeleteFile(DataFile) and files discovered + /// during FilterManifests() that were deleted by path, partition, or row-filter + /// matching. Used by higher-level operations (e.g. RowDelta) to enumerate the + /// deleted data files for delete-file cleanup. + const DataFileSet& FilesToBeDeleted() const; + + /// \brief Register a partition for dropping. + /// + /// Any manifest entry whose (spec_id, partition) pair matches will be marked DELETED. + /// + /// \param spec_id The partition spec ID + /// \param partition The partition values to drop + void DropPartition(int32_t spec_id, PartitionValues partition); + + /// \brief Set a flag that makes FilterManifests() fail if any registered + /// delete path was not found in any manifest entry. + void FailMissingDeletePaths(); + + /// \brief Set a flag that makes FilterManifests() return an error if any + /// manifest entry matches a delete condition. + void FailAnyDelete(); + + /// \brief Returns the number of manifests rewritten (replaced) by the last + /// FilterManifests() call. A manifest is replaced when it contained deleted entries + /// and was rewritten with those entries marked DELETED. + int32_t ReplacedManifestsCount() const { return replaced_manifests_count_; } + + /// \brief Returns true if any delete condition has been registered. + bool ContainsDeletes() const; + + /// \brief Set the minimum data sequence number for delete files to retain. + /// + /// Only valid for ManifestContent::kDeletes managers. Delete entries whose + /// data_sequence_number is positive and less than \p sequence_number will be + /// marked DELETED. This continuously removes delete files that cannot match + /// any remaining data rows (i.e. all data written before that sequence number + /// has itself been deleted). + /// + /// \param sequence_number the inclusive lower bound; delete files older than + /// this value are dropped + void DropDeleteFilesOlderThan(int64_t sequence_number); + + /// \brief Register data files that have been removed so their dangling DVs + /// can be cleaned up. + /// + /// Only valid for ManifestContent::kDeletes managers. For each DV whose + /// referenced_data_file path appears in \p deleted_files, the DV entry is + /// marked DELETED because the data file it targets no longer exists. + /// + /// \param deleted_files set of data files that have been marked for deletion + void RemoveDanglingDeletesFor(const DataFileSet& deleted_files); + + /// \brief Apply all accumulated delete conditions to the base snapshot's manifests. + /// + /// Manifests that cannot possibly contain deleted files are returned unchanged. + /// Manifests that do contain deleted files are rewritten using writer_factory. + /// + /// \param metadata Table metadata (provides specs and schema for evaluators) + /// \param base_snapshot The snapshot whose manifests to filter (may be null) + /// \param writer_factory Factory to create new ManifestWriter instances + /// \return The filtered manifest list, or an error + Result> FilterManifests( + const TableMetadata& metadata, const std::shared_ptr& base_snapshot, + const ManifestWriterFactory& writer_factory); + + /// \brief Apply all accumulated delete conditions to the provided manifests. + /// + /// This overload accepts only the context needed for filtering. It is intended for + /// callers that already have the active schema, partition specs, and manifest list. + /// + /// \param schema Active schema to bind row-filter expressions and metrics evaluators + /// \param specs_by_id All partition specs keyed by spec ID + /// \param manifests Manifest descriptors to filter + /// \param writer_factory Factory to create new ManifestWriter instances + /// \return The filtered manifest list, or an error + Result> FilterManifests( + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + const std::vector& manifests, + const ManifestWriterFactory& writer_factory); + + private: + /// \brief Returns true if the manifest might contain files matching any expression. + Result CanContainExpressionDeletes(const ManifestFile& manifest, + const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id); + + /// \brief Returns true if the manifest might contain files in a dropped partition. + /// + /// Checks whether the manifest's partition_spec_id matches any spec_id registered + /// via DropPartition(). Manifests from a different spec cannot contain the dropped + /// partition values. + Result CanContainDroppedPartitions(const ManifestFile& manifest) const; + + /// \brief Returns true if the manifest might contain path-deleted files. + Result CanContainDroppedFiles(const ManifestFile& manifest) const; + + /// \brief Returns true if the manifest possibly contains any deleted file. + Result CanContainDeletedFiles(const ManifestFile& manifest, + const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + bool trust_manifest_references); + + bool CanTrustManifestReferences( + const std::vector& manifests) const; + + Result FilterManifest(const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + const ManifestFile& manifest, + bool trust_manifest_references, + const ManifestWriterFactory& writer_factory, + std::unordered_set& found_paths); + + Result ManifestHasDeletedFiles(const std::vector& entries, + const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + int32_t manifest_spec_id); + + Result FilterManifestWithDeletedFiles( + const std::vector& entries, int32_t manifest_spec_id, + const std::shared_ptr& schema, const PartitionSpecsById& specs_by_id, + const ManifestWriterFactory& writer_factory, + std::unordered_set& found_paths); + + Status ValidateRequiredDeletes( + const std::unordered_set& found_paths) const; + + /// \brief Get or create a ManifestEvaluator for the given spec. + Result GetManifestEvaluator(const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + int32_t spec_id); + + /// \brief Get or create a ResidualEvaluator for the given spec. + Result GetResidualEvaluator(const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + int32_t spec_id); + + /// \brief Check whether a single entry should be deleted. + Result ShouldDelete(const ManifestEntry& entry, + const std::shared_ptr& schema, + const PartitionSpecsById& specs_by_id, + int32_t manifest_spec_id); + + const ManifestContent manifest_content_; + std::shared_ptr file_io_; + + std::shared_ptr delete_expr_; + std::unordered_set delete_paths_; + DataFileSet delete_files_; + PartitionSet drop_partitions_; + bool fail_missing_delete_paths_{false}; + bool fail_any_delete_{false}; + bool case_sensitive_{true}; + + int32_t replaced_manifests_count_{0}; + + // minimum data sequence number; delete entries older than this are dropped + int64_t min_sequence_number_{0}; + // paths of data files that were removed; DVs referencing these are dangling + std::unordered_set removed_data_file_paths_; + + std::unordered_map> + manifest_evaluator_cache_; + std::unordered_map> + residual_evaluator_cache_; +}; + +} // namespace iceberg diff --git a/src/iceberg/manifest/manifest_group.cc b/src/iceberg/manifest/manifest_group.cc index 220b8585c..8af717b25 100644 --- a/src/iceberg/manifest/manifest_group.cc +++ b/src/iceberg/manifest/manifest_group.cc @@ -262,14 +262,8 @@ Result> ManifestGroup::Entries() { Result> ManifestGroup::MakeReader( const ManifestFile& manifest) { - auto spec_it = specs_by_id_.find(manifest.partition_spec_id); - if (spec_it == specs_by_id_.end()) { - return InvalidArgument("Partition spec {} not found for manifest {}", - manifest.partition_spec_id, manifest.manifest_path); - } - ICEBERG_ASSIGN_OR_RAISE(auto reader, - ManifestReader::Make(manifest, io_, schema_, spec_it->second)); + ManifestReader::Make(manifest, io_, schema_, specs_by_id_)); reader->FilterRows(data_filter_) .FilterPartitions(partition_filter_) diff --git a/src/iceberg/manifest/manifest_merge_manager.cc b/src/iceberg/manifest/manifest_merge_manager.cc new file mode 100644 index 000000000..aedcea735 --- /dev/null +++ b/src/iceberg/manifest/manifest_merge_manager.cc @@ -0,0 +1,195 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/manifest/manifest_merge_manager.h" + +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/table_metadata.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +ManifestMergeManager::ManifestMergeManager(int64_t target_size_bytes, + int32_t min_count_to_merge, bool merge_enabled) + : target_size_bytes_(target_size_bytes), + min_count_to_merge_(min_count_to_merge), + merge_enabled_(merge_enabled) {} + +Result> ManifestMergeManager::MergeManifests( + const std::vector& existing_manifests, + const std::vector& new_manifests, int64_t snapshot_id, + const TableMetadata& metadata, std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory) { + // Combine new then existing (new-first ordering is preserved in output). + auto to_manifest_ptr = [](const ManifestFile& manifest) { return &manifest; }; + auto manifest_ranges = std::array{ + new_manifests | std::views::transform(to_manifest_ptr), + existing_manifests | std::views::transform(to_manifest_ptr), + }; + + std::vector all; + all.reserve(new_manifests.size() + existing_manifests.size()); + std::ranges::copy(manifest_ranges | std::views::join, std::back_inserter(all)); + + if (all.empty() || !merge_enabled_) { + replaced_manifests_count_ = 0; + return all | + std::views::transform([](const ManifestFile* manifest) { return *manifest; }) | + std::ranges::to>(); + } + + // Track the first (newest) manifest independently per content type. + std::map first_by_content; + std::ranges::for_each(all, [&first_by_content](const ManifestFile* manifest) { + first_by_content.try_emplace(manifest->content, manifest); + }); + + // Group manifests by (partition_spec_id, content), never merging across specs or + // content types. Reverse spec ordering preserves v3 first-row-id assignment order. + using GroupKey = std::pair; + auto group_key = [](const ManifestFile* manifest) { + return GroupKey{manifest->partition_spec_id, manifest->content}; + }; + + std::map, std::greater<>> by_spec; + std::ranges::for_each(all, [&by_spec, &group_key](const ManifestFile* manifest) { + by_spec[group_key(manifest)].push_back(manifest); + }); + + std::vector result; + result.reserve(all.size()); + replaced_manifests_count_ = 0; + for (auto& [key, group] : by_spec) { + const auto* first = first_by_content.at(key.second); + ICEBERG_ASSIGN_OR_RAISE(auto merged, MergeGroup(group, first, snapshot_id, metadata, + file_io, writer_factory)); + std::ranges::move(merged, std::back_inserter(result)); + } + return result; +} + +Result> ManifestMergeManager::MergeGroup( + const std::vector& group, const ManifestFile* first, + int64_t snapshot_id, const TableMetadata& metadata, std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory) { + // Match packEnd(group, ManifestFile::length) with lookback 1: + // 1. Process manifests in reverse order (oldest-first). + // 2. Greedy forward-pack with lookback=1: emit the current bin when the next item + // doesn't fit, then start a new bin. + // 3. Reverse each bin (restoring original item order within a bin). + // 4. Reverse the bin list (newest manifest's bin ends up first). + // Effect: the newest manifest is in the first, possibly under-filled, bin. + std::vector> bins; + std::vector current_bin; + int64_t bin_size = 0; + + for (const auto* manifest : std::views::reverse(group)) { + if (!current_bin.empty() && + bin_size + manifest->manifest_length > target_size_bytes_) { + bins.push_back(std::move(current_bin)); + current_bin.clear(); + bin_size = 0; + } + current_bin.push_back(manifest); + bin_size += manifest->manifest_length; + } + if (!current_bin.empty()) { + bins.push_back(std::move(current_bin)); + } + + for (auto& bin : bins) { + std::ranges::reverse(bin); + } + std::ranges::reverse(bins); + + // Process each bin: if the bin contains the newest manifest and is too small, + // pass its contents through unchanged. + std::vector result; + result.reserve(group.size()); + // TODO(Guotao): Flush independent bins in parallel and cache successful merged bins + // for commit retries. + for (auto& bin : bins) { + bool contains_first = std::ranges::find(bin, first) != bin.end(); + if (contains_first && std::cmp_less(bin.size(), min_count_to_merge_)) { + for (const auto* manifest : bin) { + result.push_back(*manifest); + } + } else { + ICEBERG_ASSIGN_OR_RAISE( + auto merged, FlushBin(bin, snapshot_id, metadata, file_io, writer_factory)); + // Each manifest consumed into the merged output (beyond the 1 output) is replaced. + replaced_manifests_count_ += static_cast(bin.size()) - 1; + result.push_back(std::move(merged)); + } + } + + return result; +} + +Result ManifestMergeManager::FlushBin( + const std::vector& bin, int64_t snapshot_id, + const TableMetadata& metadata, std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory) { + // A single-manifest bin requires no merging. + if (bin.size() == 1) return *bin[0]; + + const ManifestFile& first = *bin[0]; + int32_t spec_id = first.partition_spec_id; + + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto spec, metadata.PartitionSpecById(spec_id)); + + ICEBERG_ASSIGN_OR_RAISE(auto writer, writer_factory(spec_id, first.content)); + + for (const auto* manifest : bin) { + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(*manifest, file_io, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + for (const auto& entry : entries) { + bool is_current = + entry.snapshot_id.has_value() && entry.snapshot_id.value() == snapshot_id; + if (entry.status == ManifestStatus::kDeleted) { + // Carry forward only the current snapshot's deletes; drop older tombstones. + if (is_current) { + ICEBERG_RETURN_UNEXPECTED(writer->WriteDeletedEntry(entry)); + } + } else if (entry.status == ManifestStatus::kAdded && is_current) { + // Files added by the current snapshot retain their ADDED status. + ICEBERG_RETURN_UNEXPECTED(writer->WriteAddedEntry(entry)); + } else { + // Files added by prior snapshots (ADDED or EXISTING) become EXISTING. + ICEBERG_RETURN_UNEXPECTED(writer->WriteExistingEntry(entry)); + } + } + } + + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + return writer->ToManifestFile(); +} + +} // namespace iceberg diff --git a/src/iceberg/manifest/manifest_merge_manager.h b/src/iceberg/manifest/manifest_merge_manager.h new file mode 100644 index 000000000..614ab61c6 --- /dev/null +++ b/src/iceberg/manifest/manifest_merge_manager.h @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/manifest/manifest_merge_manager.h +/// Merges small manifests into fewer larger ones according to table properties. + +#include +#include +#include + +#include "iceberg/iceberg_export.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" + +namespace iceberg { + +/// \brief Merges small manifests into larger ones using greedy bin-packing. +/// +/// Manifests are grouped by partition_spec_id before merging; manifests with +/// different spec IDs are never merged together. Within a group, manifests are +/// accumulated into bins until a bin would exceed target_size_bytes, at which +/// point the bin is flushed (written) and a new one started. Manifests already +/// larger than target_size_bytes pass through unchanged. +/// +/// \note This class is non-copyable and non-movable. +class ICEBERG_EXPORT ManifestMergeManager { + public: + /// \brief Construct a merge manager with the given configuration. + /// + /// \param target_size_bytes Target output manifest size in bytes + /// \param min_count_to_merge Minimum number of manifests before any merging occurs + /// \param merge_enabled Whether merging is enabled at all + ManifestMergeManager(int64_t target_size_bytes, int32_t min_count_to_merge, + bool merge_enabled); + + ManifestMergeManager(const ManifestMergeManager&) = delete; + ManifestMergeManager& operator=(const ManifestMergeManager&) = delete; + + /// \brief Merge existing and new manifests according to configured thresholds. + /// + /// Manifests are grouped by (partition_spec_id, content) — data and delete manifests + /// are never merged together. Within each group, a greedy bin-packing algorithm + /// combines manifests up to target_size_bytes. The bin that contains the newest + /// manifest for that content type is protected by min_count_to_merge: if it has fewer + /// than that many items it is passed through unchanged. + /// + /// \note Retry and rollback cleanup are handled by the caller that owns created + /// manifest paths. + /// TODO(Guotao): Add explicit replaced-manifest tracking here if callers need direct + /// access. + /// + /// \param existing_manifests Manifests already in the base snapshot + /// \param new_manifests Newly written manifests to incorporate + /// \param snapshot_id The ID of the snapshot being committed. Used to preserve + /// ADDED/DELETED status for entries written by this snapshot and to suppress + /// stale DELETED tombstones from prior snapshots. + /// \param metadata Table metadata (provides specs and schema for readers) + /// \param file_io File IO used to open existing manifests for reading + /// \param writer_factory Factory to create new ManifestWriter instances + /// \return The merged manifest list, or an error + Result> MergeManifests( + const std::vector& existing_manifests, + const std::vector& new_manifests, int64_t snapshot_id, + const TableMetadata& metadata, std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory); + + /// \brief Returns the number of manifests replaced (consumed into merged outputs) + /// by the last MergeManifests() call. + int32_t ReplacedManifestsCount() const { return replaced_manifests_count_; } + + private: + /// \brief Merge a group of manifests sharing the same spec_id. + /// + /// \param first The overall first (newest) manifest across all groups, used to + /// apply the min_count_to_merge threshold on the bin that contains it. + Result> MergeGroup( + const std::vector& group, const ManifestFile* first, + int64_t snapshot_id, const TableMetadata& metadata, std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory); + + /// \brief Write a merged manifest from all manifests in a bin. + /// + /// Entries are written snapshot-aware: + /// - ADDED from snapshot_id → WriteAddedEntry (preserve status) + /// - DELETED from snapshot_id → WriteDeletedEntry (preserve tombstone) + /// - DELETED from older snapshots → dropped (stale tombstones are not carried forward) + /// - All other entries → WriteExistingEntry + Result FlushBin(const std::vector& bin, + int64_t snapshot_id, const TableMetadata& metadata, + std::shared_ptr file_io, + const ManifestWriterFactory& writer_factory); + + const int64_t target_size_bytes_; + const int32_t min_count_to_merge_; + const bool merge_enabled_; + int32_t replaced_manifests_count_{0}; +}; + +} // namespace iceberg diff --git a/src/iceberg/manifest/manifest_reader.cc b/src/iceberg/manifest/manifest_reader.cc index 53100b236..7747e2be3 100644 --- a/src/iceberg/manifest/manifest_reader.cc +++ b/src/iceberg/manifest/manifest_reader.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -998,6 +999,19 @@ Result> ManifestReader::Make( manifest.first_row_id); } +Result> ManifestReader::Make( + const ManifestFile& manifest, std::shared_ptr file_io, + std::shared_ptr schema, + const std::unordered_map>& specs_by_id) { + auto spec_it = specs_by_id.find(manifest.partition_spec_id); + if (spec_it == specs_by_id.end() || spec_it->second == nullptr) { + return InvalidArgument("Partition spec {} not found for manifest {}", + manifest.partition_spec_id, manifest.manifest_path); + } + auto spec = spec_it->second; + return Make(manifest, std::move(file_io), std::move(schema), std::move(spec)); +} + Result> ManifestReader::Make( std::string_view manifest_location, std::optional manifest_length, std::shared_ptr file_io, std::shared_ptr schema, diff --git a/src/iceberg/manifest/manifest_reader.h b/src/iceberg/manifest/manifest_reader.h index 1a1420216..42c56e1c2 100644 --- a/src/iceberg/manifest/manifest_reader.h +++ b/src/iceberg/manifest/manifest_reader.h @@ -92,6 +92,17 @@ class ICEBERG_EXPORT ManifestReader { const ManifestFile& manifest, std::shared_ptr file_io, std::shared_ptr schema, std::shared_ptr spec); + /// \brief Creates a reader for a manifest file using specs keyed by ID. + /// \param manifest A ManifestFile object containing metadata about the manifest. + /// \param file_io File IO implementation to use. + /// \param schema Schema used to bind the partition type. + /// \param specs_by_id Mapping of partition spec ID to PartitionSpec. + /// \return A Result containing the reader or an error. + static Result> Make( + const ManifestFile& manifest, std::shared_ptr file_io, + std::shared_ptr schema, + const std::unordered_map>& specs_by_id); + /// \brief Creates a reader for a manifest file. /// \param manifest_location Path to the manifest file. /// \param manifest_length Length of the manifest file. diff --git a/src/iceberg/manifest/manifest_writer.h b/src/iceberg/manifest/manifest_writer.h index cc57f25fc..0eaf478d0 100644 --- a/src/iceberg/manifest/manifest_writer.h +++ b/src/iceberg/manifest/manifest_writer.h @@ -22,6 +22,7 @@ /// \file iceberg/manifest/manifest_writer.h /// Data writer interface for manifest files and manifest list files. +#include #include #include #include @@ -163,6 +164,10 @@ class ICEBERG_EXPORT ManifestWriter { std::unique_ptr partition_summary_; }; +/// \brief Factory type for creating ManifestWriter instances. +using ManifestWriterFactory = std::function>( + int32_t spec_id, ManifestContent content)>; + /// \brief Write manifest files to a manifest list file. class ICEBERG_EXPORT ManifestListWriter { public: diff --git a/src/iceberg/manifest/meson.build b/src/iceberg/manifest/meson.build index 41e685ffc..d4b039a67 100644 --- a/src/iceberg/manifest/meson.build +++ b/src/iceberg/manifest/meson.build @@ -18,8 +18,10 @@ install_headers( [ 'manifest_entry.h', + 'manifest_filter_manager.h', 'manifest_group.h', 'manifest_list.h', + 'manifest_merge_manager.h', 'manifest_reader.h', 'manifest_writer.h', 'rolling_manifest_writer.h', diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index c2947f3fe..4c28bdbc2 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -67,8 +67,10 @@ iceberg_sources = files( 'location_provider.cc', 'manifest/manifest_adapter.cc', 'manifest/manifest_entry.cc', + 'manifest/manifest_filter_manager.cc', 'manifest/manifest_group.cc', 'manifest/manifest_list.cc', + 'manifest/manifest_merge_manager.cc', 'manifest/manifest_reader.cc', 'manifest/manifest_util.cc', 'manifest/manifest_writer.cc', @@ -108,6 +110,7 @@ iceberg_sources = files( 'type.cc', 'update/expire_snapshots.cc', 'update/fast_append.cc', + 'update/merging_snapshot_update.cc', 'update/pending_update.cc', 'update/set_snapshot.cc', 'update/snapshot_manager.cc', @@ -142,6 +145,7 @@ iceberg_sources = files( iceberg_data_sources = files( 'data/data_writer.cc', + 'data/delete_filter.cc', 'data/delete_loader.cc', 'data/equality_delete_writer.cc', 'data/position_delete_writer.cc', diff --git a/src/iceberg/metadata_columns.h b/src/iceberg/metadata_columns.h index 61f07c488..b390a50e8 100644 --- a/src/iceberg/metadata_columns.h +++ b/src/iceberg/metadata_columns.h @@ -50,7 +50,7 @@ struct ICEBERG_EXPORT MetadataColumns { constexpr static int32_t kIsDeletedColumnId = kInt32Max - 3; inline static const SchemaField kIsDeleted = SchemaField::MakeRequired( - kIsDeletedColumnId, "_deleted", binary(), "Whether the row has been deleted"); + kIsDeletedColumnId, "_deleted", boolean(), "Whether the row has been deleted"); constexpr static int32_t kSpecIdColumnId = kInt32Max - 4; inline static const SchemaField kSpecId = diff --git a/src/iceberg/parquet/parquet_writer.cc b/src/iceberg/parquet/parquet_writer.cc index 7e2d3d151..c70d3310c 100644 --- a/src/iceberg/parquet/parquet_writer.cc +++ b/src/iceberg/parquet/parquet_writer.cc @@ -20,9 +20,11 @@ #include "iceberg/parquet/parquet_writer.h" #include +#include #include #include +#include #include #include #include @@ -62,6 +64,14 @@ Result<::arrow::Compression::type> ParseCompression(const WriterProperties& prop } } +Status CheckCompressionAvailable(std::string_view compression_name, + ::arrow::Compression::type compression) { + ICEBERG_PRECHECK(::arrow::util::Codec::IsAvailable(compression), + "Parquet compression codec {} is not available in the current build", + compression_name); + return {}; +} + Result> ParseCodecLevel(const WriterProperties& properties) { auto level_str = properties.Get(WriterProperties::kParquetCompressionLevel); if (level_str.empty()) { @@ -98,6 +108,9 @@ class ParquetWriter::Impl { auto schema_node = std::static_pointer_cast<::parquet::schema::GroupNode>( schema_descriptor->schema_root()); + ICEBERG_RETURN_UNEXPECTED(CheckCompressionAvailable( + options.properties.Get(WriterProperties::kParquetCompression), compression)); + ICEBERG_ASSIGN_OR_RAISE(output_stream_, OpenOutputStream(options)); auto file_writer = ::parquet::ParquetFileWriter::Open( output_stream_, std::move(schema_node), std::move(writer_properties), diff --git a/src/iceberg/schema.cc b/src/iceberg/schema.cc index 00905378a..5fdd47998 100644 --- a/src/iceberg/schema.cc +++ b/src/iceberg/schema.cc @@ -40,6 +40,8 @@ Schema::Schema(std::vector fields, int32_t schema_id) schema_id_(schema_id), cache_(std::make_unique(this)) {} +Schema::~Schema() = default; + Result> Schema::Make(std::vector fields, int32_t schema_id, std::vector identifier_field_ids) { diff --git a/src/iceberg/schema.h b/src/iceberg/schema.h index 3c84bc2af..791ed5c8f 100644 --- a/src/iceberg/schema.h +++ b/src/iceberg/schema.h @@ -57,6 +57,8 @@ class ICEBERG_EXPORT Schema : public StructType { explicit Schema(std::vector fields, int32_t schema_id = kInitialSchemaId); + ~Schema() override; + /// \brief Create a schema. /// /// \param fields The fields that make up the schema. diff --git a/src/iceberg/snapshot.cc b/src/iceberg/snapshot.cc index 1b3182fd9..e51ec52f1 100644 --- a/src/iceberg/snapshot.cc +++ b/src/iceberg/snapshot.cc @@ -441,6 +441,10 @@ void SnapshotSummaryBuilder::Clear() { metrics_.Clear(); deleted_duplicate_files_ = 0; trust_partition_metrics_ = true; + manifests_counts_set_ = false; + manifests_created_ = 0; + manifests_kept_ = 0; + manifests_replaced_ = 0; } void SnapshotSummaryBuilder::SetPartitionSummaryLimit(int32_t max) { @@ -475,6 +479,14 @@ void SnapshotSummaryBuilder::Set(const std::string& property, const std::string& properties_[property] = value; } +void SnapshotSummaryBuilder::SetManifestCounts(int32_t created, int32_t kept, + int32_t replaced) { + manifests_counts_set_ = true; + manifests_created_ = created; + manifests_kept_ = kept; + manifests_replaced_ = replaced; +} + void SnapshotSummaryBuilder::Merge(const SnapshotSummaryBuilder& other) { for (const auto& [key, value] : other.properties_) { properties_[key] = value; @@ -504,6 +516,16 @@ std::unordered_map SnapshotSummaryBuilder::Build() con SetIf(deleted_duplicate_files_ > 0, builder, SnapshotSummaryFields::kDeletedDuplicatedFiles, deleted_duplicate_files_); + // Always emit all three manifest count fields together when they have been set, + // matching Java's SnapshotProducer.buildManifestCountSummary which sets them + // unconditionally. + SetIf(manifests_counts_set_, builder, SnapshotSummaryFields::kManifestsCreated, + manifests_created_); + SetIf(manifests_counts_set_, builder, SnapshotSummaryFields::kManifestsKept, + manifests_kept_); + SetIf(manifests_counts_set_, builder, SnapshotSummaryFields::kManifestsReplaced, + manifests_replaced_); + SetIf(trust_partition_metrics_, builder, SnapshotSummaryFields::kChangedPartitionCountProp, partition_metrics_.size()); diff --git a/src/iceberg/snapshot.h b/src/iceberg/snapshot.h index f3e7ffb85..178c21dd7 100644 --- a/src/iceberg/snapshot.h +++ b/src/iceberg/snapshot.h @@ -338,6 +338,15 @@ class ICEBERG_EXPORT SnapshotSummaryBuilder { /// \param value Property value void Set(const std::string& property, const std::string& value); + /// \brief Set manifest count summary fields. + /// + /// Records how many manifests were created, kept, and replaced in this snapshot. + /// + /// \param created Manifests written by this snapshot + /// \param kept Manifests carried over unchanged from the previous snapshot + /// \param replaced Manifests rewritten or merged away + void SetManifestCounts(int32_t created, int32_t kept, int32_t replaced); + /// \brief Merge another builder's metrics into this one /// /// \param other The builder to merge from @@ -359,6 +368,10 @@ class ICEBERG_EXPORT SnapshotSummaryBuilder { int32_t max_changed_partitions_for_summaries_{0}; int64_t deleted_duplicate_files_{0}; bool trust_partition_metrics_{true}; + bool manifests_counts_set_{false}; + int32_t manifests_created_{0}; + int32_t manifests_kept_{0}; + int32_t manifests_replaced_{0}; }; /// \brief Data operation that produce snapshots. diff --git a/src/iceberg/table_scan.cc b/src/iceberg/table_scan.cc index f61bd3a0c..71075d90a 100644 --- a/src/iceberg/table_scan.cc +++ b/src/iceberg/table_scan.cc @@ -430,7 +430,7 @@ TableScanBuilder& TableScanBuilder::UseRef(const std::string auto iter = metadata_->refs.find(ref); ICEBERG_BUILDER_CHECK(iter != metadata_->refs.end(), "Cannot find ref {}", ref); ICEBERG_BUILDER_CHECK(iter->second != nullptr, "Ref {} is null", ref); - int32_t snapshot_id = iter->second->snapshot_id; + const int64_t snapshot_id = iter->second->snapshot_id; ICEBERG_BUILDER_ASSIGN_OR_RETURN(std::ignore, metadata_->SnapshotById(snapshot_id)); context_.snapshot_id = snapshot_id; diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 1d80b29a5..0b7dc64b8 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -124,6 +124,7 @@ add_iceberg_test(util_test data_file_set_test.cc decimal_test.cc endian_test.cc + file_io_test.cc formatter_test.cc lazy_test.cc location_util_test.cc @@ -180,6 +181,7 @@ if(ICEBERG_BUILD_BUNDLE) delete_file_index_test.cc manifest_group_test.cc manifest_list_versions_test.cc + manifest_merge_manager_test.cc manifest_reader_stats_test.cc manifest_reader_test.cc manifest_writer_versions_test.cc @@ -205,6 +207,8 @@ if(ICEBERG_BUILD_BUNDLE) SOURCES expire_snapshots_test.cc fast_append_test.cc + manifest_filter_manager_test.cc + merging_snapshot_update_test.cc name_mapping_update_test.cc snapshot_manager_test.cc transaction_test.cc @@ -220,6 +224,7 @@ if(ICEBERG_BUILD_BUNDLE) USE_BUNDLE SOURCES data_writer_test.cc + delete_filter_test.cc delete_loader_test.cc) endif() diff --git a/src/iceberg/test/arrow_io_test.cc b/src/iceberg/test/arrow_io_test.cc index 0c885d07a..7edaf0756 100644 --- a/src/iceberg/test/arrow_io_test.cc +++ b/src/iceberg/test/arrow_io_test.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -341,6 +342,20 @@ TEST_F(LocalFileIOTest, DeleteFile) { EXPECT_THAT(del_res, HasErrorMessage("Cannot delete file")); } +TEST_F(LocalFileIOTest, DeleteFiles) { + auto first_path = CreateNewTempFilePath(); + auto second_path = CreateNewTempFilePath(); + ASSERT_THAT(file_io_->WriteFile(first_path, "hello"), IsOk()); + ASSERT_THAT(file_io_->WriteFile(second_path, "world"), IsOk()); + + std::vector paths = {first_path, second_path}; + EXPECT_THAT(file_io_->DeleteFiles(paths), IsOk()); + + EXPECT_THAT(file_io_->ReadFile(first_path, std::nullopt), IsError(ErrorKind::kIOError)); + EXPECT_THAT(file_io_->ReadFile(second_path, std::nullopt), + IsError(ErrorKind::kIOError)); +} + void VerifyReadFullyReadsFromAbsolutePosition(const std::shared_ptr& file_io, const std::string& path) { ASSERT_THAT(file_io->WriteFile(path, "abcdef"), IsOk()); diff --git a/src/iceberg/test/delete_filter_test.cc b/src/iceberg/test/delete_filter_test.cc new file mode 100644 index 000000000..89d1b6b85 --- /dev/null +++ b/src/iceberg/test/delete_filter_test.cc @@ -0,0 +1,1625 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/data/delete_filter.h" + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include "iceberg/arrow/arrow_io_internal.h" +#include "iceberg/data/equality_delete_writer.h" +#include "iceberg/data/position_delete_writer.h" +#include "iceberg/file_format.h" +#include "iceberg/file_reader.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/metadata_columns.h" +#include "iceberg/parquet/parquet_register.h" +#include "iceberg/partition_spec.h" +#include "iceberg/row/arrow_array_wrapper.h" +#include "iceberg/row/partition_values.h" +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/schema_internal.h" +#include "iceberg/table_metadata.h" +#include "iceberg/test/matchers.h" +#include "iceberg/type.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { + +struct ExportedBatch { + ArrowSchema schema{}; + ArrowArray array{}; + + ExportedBatch() = default; + ~ExportedBatch() { + if (array.release != nullptr) { + array.release(&array); + } + if (schema.release != nullptr) { + schema.release(&schema); + } + } + + ExportedBatch(const ExportedBatch&) = delete; + ExportedBatch& operator=(const ExportedBatch&) = delete; + + ExportedBatch(ExportedBatch&& other) noexcept + : schema(other.schema), array(other.array) { + other.schema.release = nullptr; + other.array.release = nullptr; + } + ExportedBatch& operator=(ExportedBatch&& other) noexcept = delete; +}; + +std::vector FieldNames(const Schema& schema) { + std::vector names; + for (const auto& field : schema.fields()) { + names.emplace_back(field.name()); + } + return names; +} + +std::vector FieldIds(const Schema& schema) { + std::vector ids; + for (const auto& field : schema.fields()) { + ids.push_back(field.field_id()); + } + return ids; +} + +std::vector StructFieldIds(const StructType& struct_type) { + std::vector ids; + for (const auto& field : struct_type.fields()) { + ids.push_back(field.field_id()); + } + return ids; +} + +void ExpectAliveRows(const AliveRowSelection& alive, + const std::vector& expected) { + ASSERT_EQ(alive.alive_count(), static_cast(expected.size())); + EXPECT_EQ(alive.indices, expected); +} + +class CapturingReader : public Reader { + public: + explicit CapturingReader(std::shared_ptr* projection) + : projection_(projection) {} + + Status Open(const ReaderOptions& options) override { + *projection_ = options.projection; + return {}; + } + + Status Close() override { return {}; } + + Result> Next() override { return std::nullopt; } + + Result Schema() override { + ArrowSchema schema; + ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(**projection_, &schema)); + return schema; + } + + Result> Metadata() override { + return std::unordered_map{}; + } + + private: + std::shared_ptr* projection_; +}; + +class ScopedReaderFactory { + public: + ScopedReaderFactory(FileFormatType format_type, ReaderFactory factory) + : format_type_(format_type), + previous_(ReaderFactoryRegistry::GetFactory(format_type)) { + ReaderFactoryRegistry::GetFactory(format_type_) = std::move(factory); + } + + ~ScopedReaderFactory() { + ReaderFactoryRegistry::GetFactory(format_type_) = std::move(previous_); + } + + private: + FileFormatType format_type_; + ReaderFactory previous_; +}; + +} // namespace + +class DeleteFilterTest : public ::testing::Test { + protected: + static void SetUpTestSuite() { parquet::RegisterAll(); } + + void SetUp() override { + file_io_ = arrow::ArrowFileSystemFileIO::MakeMockFileIO(); + table_schema_ = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(2, "name", string()), + SchemaField::MakeOptional(3, "category", string())}); + partition_spec_ = PartitionSpec::Unpartitioned(); + } + + std::shared_ptr Project(std::initializer_list field_ids) const { + std::unordered_set ids(field_ids.begin(), field_ids.end()); + auto result = table_schema_->Project(ids); + EXPECT_TRUE(result.has_value()) << "Projection failed: " << result.error().message; + return std::move(result.value()); + } + + Result MakeBatch(const Schema& schema, + const std::string& json_data) const { + ArrowSchema type_schema; + ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(schema, &type_schema)); + auto arrow_type_result = ::arrow::ImportType(&type_schema); + if (!arrow_type_result.ok()) { + return UnknownError(arrow_type_result.status().ToString()); + } + auto struct_type = ::arrow::struct_(arrow_type_result.MoveValueUnsafe()->fields()); + auto array_result = ::arrow::json::ArrayFromJSONString(struct_type, json_data); + if (!array_result.ok()) { + return UnknownError(array_result.status().ToString()); + } + + ExportedBatch batch; + ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(schema, &batch.schema)); + auto export_status = + ::arrow::ExportArray(*array_result.MoveValueUnsafe(), &batch.array); + if (!export_status.ok()) { + return UnknownError(export_status.ToString()); + } + return batch; + } + + Result> PositionDeleteFile( + const std::string& path, const std::vector& positions, + const std::string& data_path = std::string(kDataPath)) { + PositionDeleteWriterOptions options{ + .path = path, + .schema = table_schema_, + .spec = partition_spec_, + .partition = PartitionValues{}, + .format = FileFormatType::kParquet, + .io = file_io_, + .flush_threshold = 10000, + .properties = {{"write.parquet.compression-codec", "uncompressed"}}, + }; + + ICEBERG_ASSIGN_OR_RAISE(auto writer, PositionDeleteWriter::Make(options)); + for (int64_t pos : positions) { + ICEBERG_RETURN_UNEXPECTED(writer->WriteDelete(data_path, pos)); + } + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + ICEBERG_ASSIGN_OR_RAISE(auto metadata, writer->Metadata()); + return metadata.data_files[0]; + } + + Result> EqualityDeleteFile( + const std::string& path, const std::string& json_data, + std::vector equality_field_ids) { + EqualityDeleteWriterOptions options{ + .path = path, + .schema = table_schema_, + .spec = partition_spec_, + .partition = PartitionValues{}, + .format = FileFormatType::kParquet, + .io = file_io_, + .equality_field_ids = std::move(equality_field_ids), + .properties = {{"write.parquet.compression-codec", "uncompressed"}}, + }; + + ICEBERG_ASSIGN_OR_RAISE(auto writer, EqualityDeleteWriter::Make(options)); + ICEBERG_ASSIGN_OR_RAISE(auto batch, MakeBatch(*table_schema_, json_data)); + ICEBERG_RETURN_UNEXPECTED(writer->Write(&batch.array)); + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + ICEBERG_ASSIGN_OR_RAISE(auto metadata, writer->Metadata()); + return metadata.data_files[0]; + } + + static constexpr std::string_view kDataPath = "data.parquet"; + + std::shared_ptr file_io_; + std::shared_ptr table_schema_; + std::shared_ptr partition_spec_; +}; + +enum class RequiredSchemaRequest { + kProjectFields, + kIdAndRowPos, +}; + +struct RequiredSchemaCase { + const char* name; + RequiredSchemaRequest request; + std::vector requested_field_ids; + std::vector> equality_ids_by_file; + bool has_pos_delete; + bool need_row_pos_col; + std::vector expected_field_ids; + std::vector expected_field_names; + bool expected_has_position_deletes; + bool expected_has_equality_deletes; +}; + +template +std::string ParamName(const testing::TestParamInfo& info) { + return info.param.name; +} + +class DeleteFilterRequiredSchemaTest + : public DeleteFilterTest, + public testing::WithParamInterface { + protected: + std::shared_ptr RequestedSchema(const RequiredSchemaCase& test_case) { + switch (test_case.request) { + case RequiredSchemaRequest::kProjectFields: { + std::unordered_set ids(test_case.requested_field_ids.begin(), + test_case.requested_field_ids.end()); + auto result = table_schema_->Project(ids); + EXPECT_TRUE(result.has_value()) + << "Projection failed: " << result.error().message; + return std::move(result.value()); + } + case RequiredSchemaRequest::kIdAndRowPos: + return std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), MetadataColumns::kRowPosition}); + } + return nullptr; + } + + std::vector> DeleteFiles( + const RequiredSchemaCase& test_case) { + std::vector> delete_files; + for (size_t index = 0; index < test_case.equality_ids_by_file.size(); ++index) { + delete_files.push_back(std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = std::format("{}-eq-{}.parquet", test_case.name, index), + .file_format = FileFormatType::kParquet, + .equality_ids = test_case.equality_ids_by_file[index], + })); + } + if (test_case.has_pos_delete) { + delete_files.push_back(std::make_shared(DataFile{ + .content = DataFile::Content::kPositionDeletes, + .file_path = std::format("{}-pos.parquet", test_case.name), + .file_format = FileFormatType::kParquet, + })); + } + return delete_files; + } +}; + +TEST_P(DeleteFilterRequiredSchemaTest, ComputesRequiredSchema) { + const auto& test_case = GetParam(); + auto delete_files = DeleteFiles(test_case); + auto requested_schema = RequestedSchema(test_case); + + auto filter = + DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_, test_case.need_row_pos_col); + + ASSERT_THAT(filter, IsOk()); + EXPECT_EQ(filter.value()->HasPositionDeletes(), + test_case.expected_has_position_deletes); + EXPECT_EQ(filter.value()->HasEqualityDeletes(), + test_case.expected_has_equality_deletes); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), + testing::ElementsAreArray(test_case.expected_field_ids)); + EXPECT_THAT(FieldNames(*filter.value()->RequiredSchema()), + testing::ElementsAreArray(test_case.expected_field_names)); +} + +INSTANTIATE_TEST_SUITE_P( + RequiredSchema, DeleteFilterRequiredSchemaTest, + testing::Values( + RequiredSchemaCase{ + .name = "UnchangedWithoutDeletes", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {2, 1}, + .equality_ids_by_file = {}, + .has_pos_delete = false, + .need_row_pos_col = true, + .expected_field_ids = {1, 2}, + .expected_field_names = {"id", "name"}, + .expected_has_position_deletes = false, + .expected_has_equality_deletes = false, + }, + RequiredSchemaCase{ + .name = "AddsEqualityFieldsAndRowPos", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {1}, + .equality_ids_by_file = {{2}, {3, 1}}, + .has_pos_delete = true, + .need_row_pos_col = true, + .expected_field_ids = {1, 2, 3, MetadataColumns::kFilePositionColumnId}, + .expected_field_names = {"id", "name", "category", + std::string(MetadataColumns::kRowPosition.name())}, + .expected_has_position_deletes = true, + .expected_has_equality_deletes = true, + }, + RequiredSchemaCase{ + .name = "AddsEqualityFieldsInDeclaredOrder", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {1}, + .equality_ids_by_file = {{3, 2}}, + .has_pos_delete = false, + .need_row_pos_col = true, + .expected_field_ids = {1, 3, 2}, + .expected_field_names = {"id", "category", "name"}, + .expected_has_position_deletes = false, + .expected_has_equality_deletes = true, + }, + RequiredSchemaCase{ + .name = "DeduplicatesRowPos", + .request = RequiredSchemaRequest::kIdAndRowPos, + .requested_field_ids = {}, + .equality_ids_by_file = {}, + .has_pos_delete = true, + .need_row_pos_col = true, + .expected_field_ids = {1, MetadataColumns::kFilePositionColumnId}, + .expected_field_names = {"id", + std::string(MetadataColumns::kRowPosition.name())}, + .expected_has_position_deletes = true, + .expected_has_equality_deletes = false, + }, + RequiredSchemaCase{ + .name = "NeedRowPosColFalseOmitsPos", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {1}, + .equality_ids_by_file = {}, + .has_pos_delete = true, + .need_row_pos_col = false, + .expected_field_ids = {1}, + .expected_field_names = {"id"}, + .expected_has_position_deletes = true, + .expected_has_equality_deletes = false, + }, + RequiredSchemaCase{ + .name = "NeedRowPosColTrueAppendsPos", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {1}, + .equality_ids_by_file = {}, + .has_pos_delete = true, + .need_row_pos_col = true, + .expected_field_ids = {1, MetadataColumns::kFilePositionColumnId}, + .expected_field_names = {"id", + std::string(MetadataColumns::kRowPosition.name())}, + .expected_has_position_deletes = true, + .expected_has_equality_deletes = false, + }, + RequiredSchemaCase{ + .name = "AddsFieldsInJavaOrder", + .request = RequiredSchemaRequest::kProjectFields, + .requested_field_ids = {1}, + .equality_ids_by_file = {{2}, {3}}, + .has_pos_delete = true, + .need_row_pos_col = true, + .expected_field_ids = {1, 2, 3, MetadataColumns::kFilePositionColumnId}, + .expected_field_names = {"id", "name", "category", + std::string(MetadataColumns::kRowPosition.name())}, + .expected_has_position_deletes = true, + .expected_has_equality_deletes = true, + }), + ParamName); + +TEST_F(DeleteFilterTest, EqualityFieldsCanBeTopLevelPrimitiveOrNestedPrimitive) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", struct_({SchemaField::MakeOptional(5, "city", string())}))}); + auto requested_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + auto eq_by_struct = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-id.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {1}, + }); + + std::vector> top_level_primitive_delete = {eq_by_struct}; + auto top_level_filter = + DeleteFilter::Make(std::string(kDataPath), top_level_primitive_delete, + nested_schema, requested_schema, file_io_); + ASSERT_THAT(top_level_filter, IsOk()); + EXPECT_THAT(FieldIds(*top_level_filter.value()->RequiredSchema()), + testing::ElementsAre(1)); + + auto eq_by_nested_field = std::make_shared(*eq_by_struct); + eq_by_nested_field->equality_ids = {5}; + std::vector> nested_delete = {eq_by_nested_field}; + + auto nested_filter = DeleteFilter::Make(std::string(kDataPath), nested_delete, + nested_schema, requested_schema, file_io_); + + ASSERT_THAT(nested_filter, IsOk()); + EXPECT_THAT(FieldIds(*nested_filter.value()->RequiredSchema()), + testing::ElementsAre(1, 4)); +} + +TEST_F(DeleteFilterTest, RequiredSchemaMergesNestedSibling) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}); + auto requested_schema = std::shared_ptr( + nested_schema->Project(std::unordered_set{1, 5}).value()); + auto eq_by_state = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-state.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {6}, + }); + std::vector> delete_files = {eq_by_state}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + + ASSERT_THAT(filter, IsOk()); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 4)); + const auto& info = filter.value()->RequiredSchema()->fields()[1]; + auto info_type = std::dynamic_pointer_cast(info.type()); + ASSERT_NE(info_type, nullptr); + EXPECT_THAT(StructFieldIds(*info_type), testing::ElementsAre(5, 6)); +} + +TEST_F(DeleteFilterTest, StructEqualityFieldErrors) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}); + auto requested_schema = std::shared_ptr( + nested_schema->Project(std::unordered_set{1, 5}).value()); + auto eq_by_info = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-info.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {4}, + }); + std::vector> delete_files = {eq_by_info}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(filter, HasErrorMessage("must reference a primitive field")); +} + +enum class AliveRowsDeleteKind { + kNone, + kPosition, + kEqualityName, + kEqualityNameAndCategory, + kMixedPositionAndEqualityId, +}; + +enum class CounterMode { + kNone, + kAttachCounter, + kNullCounter, +}; + +struct AliveRowsCase { + const char* name; + AliveRowsDeleteKind delete_kind; + std::vector position_delete_positions; + std::string position_delete_data_path; + bool need_row_pos_col; + CounterMode counter_mode; + std::string batch_json; + std::vector expected_alive_rows; + std::optional expected_delete_count; +}; + +class DeleteFilterAliveRowsTest : public DeleteFilterTest, + public testing::WithParamInterface {}; + +TEST_P(DeleteFilterAliveRowsTest, ComputesAliveRows) { + const auto& test_case = GetParam(); + std::vector> delete_files; + switch (test_case.delete_kind) { + case AliveRowsDeleteKind::kNone: + break; + case AliveRowsDeleteKind::kPosition: { + auto data_path = test_case.position_delete_data_path.empty() + ? std::string(kDataPath) + : test_case.position_delete_data_path; + ICEBERG_UNWRAP_OR_FAIL( + auto pos_delete, + PositionDeleteFile(std::format("{}-pos.parquet", test_case.name), + test_case.position_delete_positions, data_path)); + delete_files.push_back(pos_delete); + break; + } + case AliveRowsDeleteKind::kEqualityName: { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile(std::format("{}-eq-name.parquet", test_case.name), + R"([[0, "Bob", "unused"]])", {2})); + delete_files.push_back(eq_by_name); + break; + } + case AliveRowsDeleteKind::kEqualityNameAndCategory: { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile(std::format("{}-eq-name.parquet", test_case.name), + R"([[0, "Bob", "unused"]])", {2})); + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_category, + EqualityDeleteFile(std::format("{}-eq-category.parquet", test_case.name), + R"([[0, "unused", "red"]])", {3})); + delete_files.push_back(eq_by_name); + delete_files.push_back(eq_by_category); + break; + } + case AliveRowsDeleteKind::kMixedPositionAndEqualityId: { + ICEBERG_UNWRAP_OR_FAIL( + auto pos_delete, + PositionDeleteFile(std::format("{}-pos.parquet", test_case.name), + test_case.position_delete_positions)); + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_id, + EqualityDeleteFile(std::format("{}-eq-id.parquet", test_case.name), + R"([[3, "unused", "unused"]])", {1})); + delete_files.push_back(pos_delete); + delete_files.push_back(eq_by_id); + break; + } + } + + auto requested_schema = Project({1}); + std::shared_ptr counter; + if (test_case.counter_mode == CounterMode::kAttachCounter) { + counter = std::make_shared(); + } + auto filter = + DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_, test_case.need_row_pos_col, counter); + ASSERT_THAT(filter, IsOk()); + ICEBERG_UNWRAP_OR_FAIL( + auto batch, MakeBatch(*filter.value()->RequiredSchema(), test_case.batch_json)); + + auto alive = filter.value()->ComputeAliveRows(batch.schema, batch.array); + + ASSERT_THAT(alive, IsOk()); + ExpectAliveRows(alive.value(), test_case.expected_alive_rows); + if (test_case.expected_delete_count.has_value()) { + ASSERT_NE(counter, nullptr); + EXPECT_EQ(counter->Get(), test_case.expected_delete_count.value()); + } +} + +INSTANTIATE_TEST_SUITE_P( + AliveRows, DeleteFilterAliveRowsTest, + testing::Values( + AliveRowsCase{ + .name = "AllReturnedWithoutDeletes", + .delete_kind = AliveRowsDeleteKind::kNone, + .position_delete_positions = {}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = R"([[1], [2], [3]])", + .expected_alive_rows = {0, 1, 2}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "PositionDeletesFilterByRowPos", + .delete_kind = AliveRowsDeleteKind::kPosition, + .position_delete_positions = {1, 3}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = R"([[10, 0], [20, 1], [30, 2], [40, 3]])", + .expected_alive_rows = {0, 2}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "EqualityDeletesApplyOrSemantics", + .delete_kind = AliveRowsDeleteKind::kEqualityNameAndCategory, + .position_delete_positions = {}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = + R"([[1, "Alice", "blue"], [2, "Bob", "blue"], [3, "Carol", "red"], [4, "Dan", "green"]])", + .expected_alive_rows = {0, 3}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "MixedDeletesPosBeforeEqCanDeleteAll", + .delete_kind = AliveRowsDeleteKind::kMixedPositionAndEqualityId, + .position_delete_positions = {0, 1}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = R"([[1, 0], [2, 1], [3, 2]])", + .expected_alive_rows = {}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "EmptyBatchReturnsEmptyBitmap", + .delete_kind = AliveRowsDeleteKind::kNone, + .position_delete_positions = {}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = R"([])", + .expected_alive_rows = {}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "NeedRowPosColFalseSkipsPosFiltering", + .delete_kind = AliveRowsDeleteKind::kPosition, + .position_delete_positions = {0, 1}, + .position_delete_data_path = "", + .need_row_pos_col = false, + .counter_mode = CounterMode::kNone, + .batch_json = R"([[10], [20], [30]])", + .expected_alive_rows = {0, 1, 2}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "CounterCountsPosDeletes", + .delete_kind = AliveRowsDeleteKind::kPosition, + .position_delete_positions = {0, 2}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kAttachCounter, + .batch_json = R"([[10, 0], [20, 1], [30, 2], [40, 3]])", + .expected_alive_rows = {1, 3}, + .expected_delete_count = 2, + }, + AliveRowsCase{ + .name = "CounterCountsEqDeletes", + .delete_kind = AliveRowsDeleteKind::kEqualityName, + .position_delete_positions = {}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kAttachCounter, + .batch_json = R"([[1, "Alice"], [2, "Bob"], [3, "Bob"], [4, "Dan"]])", + .expected_alive_rows = {0, 3}, + .expected_delete_count = 2, + }, + AliveRowsCase{ + .name = "NullCounterIsNoOp", + .delete_kind = AliveRowsDeleteKind::kPosition, + .position_delete_positions = {0}, + .position_delete_data_path = "", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNullCounter, + .batch_json = R"([[10, 0], [20, 1]])", + .expected_alive_rows = {1}, + .expected_delete_count = std::nullopt, + }, + AliveRowsCase{ + .name = "PosDeleteOnlyFiltersMatchingPath", + .delete_kind = AliveRowsDeleteKind::kPosition, + .position_delete_positions = {0, 1, 2}, + .position_delete_data_path = "other-data.parquet", + .need_row_pos_col = true, + .counter_mode = CounterMode::kNone, + .batch_json = R"([[10, 0], [20, 1], [30, 2]])", + .expected_alive_rows = {0, 1, 2}, + .expected_delete_count = std::nullopt, + }), + ParamName); + +TEST_F(DeleteFilterTest, TopLevelStructEqualityErrors) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", struct_({SchemaField::MakeOptional(5, "city", string())}))}); + auto requested_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + + auto eq_by_info = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-info.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {4}, + }); + std::vector> delete_files = {eq_by_info}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(filter, HasErrorMessage("must reference a primitive field")); +} + +TEST_F(DeleteFilterTest, NestedStructFieldEqualityFiltersRows) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}); + auto requested_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + + EqualityDeleteWriterOptions options{ + .path = "eq-city.parquet", + .schema = nested_schema, + .spec = partition_spec_, + .partition = PartitionValues{}, + .format = FileFormatType::kParquet, + .io = file_io_, + .equality_field_ids = {5}, + .properties = {{"write.parquet.compression-codec", "uncompressed"}}, + }; + ICEBERG_UNWRAP_OR_FAIL(auto writer, EqualityDeleteWriter::Make(options)); + ICEBERG_UNWRAP_OR_FAIL( + auto delete_batch, + MakeBatch(*nested_schema, + R"([{"id": 0, "info": {"city": "Paris", "state": "FR"}}])")); + ASSERT_THAT(writer->Write(&delete_batch.array), IsOk()); + ASSERT_THAT(writer->Close(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto eq_by_city_meta, writer->Metadata()); + auto eq_by_city = eq_by_city_meta.data_files[0]; + + std::vector> delete_files = {eq_by_city}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 4)); + + ICEBERG_UNWRAP_OR_FAIL(auto data_batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([{"id": 1, "info": {"city": "London"}}, + {"id": 2, "info": {"city": "Paris"}}, + {"id": 3, "info": null}])")); + + auto alive = filter.value()->ComputeAliveRows(data_batch.schema, data_batch.array); + + ASSERT_THAT(alive, IsOk()); + ExpectAliveRows(alive.value(), {0, 2}); +} + +TEST_F(DeleteFilterTest, NestedEqualityWithPartialStructNoOverDelete) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}); + auto requested_schema = std::shared_ptr( + nested_schema->Project(std::unordered_set{1, 5}).value()); + + EqualityDeleteWriterOptions options{ + .path = "eq-state-partial.parquet", + .schema = nested_schema, + .spec = partition_spec_, + .partition = PartitionValues{}, + .format = FileFormatType::kParquet, + .io = file_io_, + .equality_field_ids = {6}, + .properties = {{"write.parquet.compression-codec", "uncompressed"}}, + }; + ICEBERG_UNWRAP_OR_FAIL(auto writer, EqualityDeleteWriter::Make(options)); + ICEBERG_UNWRAP_OR_FAIL( + auto delete_batch, + MakeBatch(*nested_schema, + R"([{"id": 0, "info": {"city": "ignored", "state": "CA"}}])")); + ASSERT_THAT(writer->Write(&delete_batch.array), IsOk()); + ASSERT_THAT(writer->Close(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto eq_by_state_meta, writer->Metadata()); + auto eq_by_state = eq_by_state_meta.data_files[0]; + + std::vector> delete_files = {eq_by_state}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto data_batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([{"id": 1, "info": {"city": "SF", "state": "CA"}}, + {"id": 2, "info": {"city": "NYC", "state": "NY"}}, + {"id": 3, "info": null}])")); + + auto alive = filter.value()->ComputeAliveRows(data_batch.schema, data_batch.array); + + ASSERT_THAT(alive, IsOk()); + ExpectAliveRows(alive.value(), {1, 2}); +} + +TEST_F(DeleteFilterTest, EqualityDeleteProjectionSortsNestedFieldsById) { + auto nested_schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(6, "state", string()), + SchemaField::MakeOptional(5, "city", string())}))}); + auto requested_schema = std::shared_ptr( + nested_schema->Project(std::unordered_set{1}).value()); + auto eq_delete = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-city-state.orc", + .file_format = FileFormatType::kOrc, + .equality_ids = {6, 5}, + }); + std::vector> delete_files = {eq_delete}; + + std::shared_ptr captured_projection; + ScopedReaderFactory reader_factory( + FileFormatType::kOrc, [&captured_projection]() -> Result> { + return std::make_unique(&captured_projection); + }); + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, nested_schema, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + ASSERT_THAT(filter.value()->EqDeletedRowFilter(), IsOk()); + + ASSERT_NE(captured_projection, nullptr); + ASSERT_EQ(captured_projection->fields().size(), 1); + auto info_type = + std::dynamic_pointer_cast(captured_projection->fields()[0].type()); + ASSERT_NE(info_type, nullptr); + EXPECT_THAT(StructFieldIds(*info_type), testing::ElementsAre(5, 6)); +} + +TEST_F(DeleteFilterTest, DroppedTopLevelFieldResolvedBySchemas) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}, + /*schema_id=*/2); + auto historic_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "dropped_value", string())}, + /*schema_id=*/1); + auto requested_schema = current_schema; + auto eq_by_dropped = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-dropped.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {7}, + }); + std::vector> delete_files = {eq_by_dropped}; + std::vector> schemas = {current_schema, historic_schema}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, current_schema, + requested_schema, file_io_, schemas); + + ASSERT_THAT(filter, IsOk()); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 7)); + EXPECT_THAT(FieldNames(*filter.value()->RequiredSchema()), + testing::ElementsAre("id", "dropped_value")); +} + +TEST_F(DeleteFilterTest, MakeFieldLookupSchemasMayIncludeCurrent) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "current_value", string())}, + /*schema_id=*/2); + auto old_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "old_value", int32())}, + /*schema_id=*/1); + std::vector> schemas = {old_schema, current_schema}; + + auto lookup_result = DeleteFilter::MakeFieldLookup(current_schema, schemas); + ASSERT_THAT(lookup_result, IsOk()); + + auto field_result = lookup_result.value()(7); + ASSERT_THAT(field_result, IsOk()); + ASSERT_TRUE(field_result.value().has_value()); + EXPECT_EQ(field_result.value()->field.name(), "current_value"); + EXPECT_EQ(field_result.value()->field.type()->type_id(), TypeId::kString); +} + +TEST_F(DeleteFilterTest, MakeFieldLookupCurrentSchemaWins) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "current_value", string())}, + /*schema_id=*/3); + auto fallback_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "fallback_value", int32())}, + /*schema_id=*/2); + std::vector> schemas = {fallback_schema}; + + auto lookup_result = DeleteFilter::MakeFieldLookup(current_schema, schemas); + ASSERT_THAT(lookup_result, IsOk()); + + auto field_result = lookup_result.value()(7); + ASSERT_THAT(field_result, IsOk()); + ASSERT_TRUE(field_result.value().has_value()); + EXPECT_EQ(field_result.value()->field.name(), "current_value"); + EXPECT_EQ(field_result.value()->field.type()->type_id(), TypeId::kString); +} + +TEST_F(DeleteFilterTest, MakeFieldLookupLatestFallbackSchemaWins) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}, + /*schema_id=*/3); + auto older_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "old_value", int32())}, + /*schema_id=*/1); + auto newer_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "new_value", string())}, + /*schema_id=*/2); + std::vector> schemas = {older_schema, newer_schema}; + + auto lookup_result = DeleteFilter::MakeFieldLookup(current_schema, schemas); + ASSERT_THAT(lookup_result, IsOk()); + + auto field_result = lookup_result.value()(7); + ASSERT_THAT(field_result, IsOk()); + ASSERT_TRUE(field_result.value().has_value()); + EXPECT_EQ(field_result.value()->field.name(), "new_value"); + EXPECT_EQ(field_result.value()->field.type()->type_id(), TypeId::kString); +} + +TEST_F(DeleteFilterTest, DroppedNestedFieldResolvedBySchemas) { + auto current_schema = std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", struct_({SchemaField::MakeOptional(5, "city", string())}))}, + /*schema_id=*/2); + auto historic_schema = std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}, + /*schema_id=*/1); + auto requested_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + auto eq_by_dropped_nested = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-dropped-state.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {6}, + }); + std::vector> delete_files = {eq_by_dropped_nested}; + std::vector> schemas = {current_schema, historic_schema}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, current_schema, + requested_schema, file_io_, schemas); + + ASSERT_THAT(filter, IsOk()); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 4)); + const auto& info = filter.value()->RequiredSchema()->fields()[1]; + auto info_type = std::dynamic_pointer_cast(info.type()); + ASSERT_NE(info_type, nullptr); + EXPECT_THAT(StructFieldIds(*info_type), testing::ElementsAre(6)); +} + +TEST_F(DeleteFilterTest, MetadataLookupUsesSchemas) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}, + /*schema_id=*/2); + auto historic_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "dropped_value", string())}, + /*schema_id=*/1); + auto metadata = std::make_shared(TableMetadata{ + .format_version = TableMetadata::kDefaultTableFormatVersion, + .schemas = {historic_schema, current_schema}, + .current_schema_id = current_schema->schema_id(), + }); + auto requested_schema = current_schema; + auto eq_by_dropped = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-dropped.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {7}, + }); + std::vector> delete_files = {eq_by_dropped}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, metadata, + requested_schema, file_io_); + + ASSERT_THAT(filter, IsOk()); + EXPECT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 7)); +} + +TEST_F(DeleteFilterTest, MetadataLookupPrefersLatestFallbackSchema) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}, + /*schema_id=*/3); + auto older_historic_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "old_name", int32())}, + /*schema_id=*/1); + auto newer_historic_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "new_name", string())}, + /*schema_id=*/2); + auto metadata = std::make_shared(TableMetadata{ + .format_version = TableMetadata::kDefaultTableFormatVersion, + .schemas = {older_historic_schema, newer_historic_schema, current_schema}, + .current_schema_id = current_schema->schema_id(), + }); + auto eq_by_dropped = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-dropped.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {7}, + }); + std::vector> delete_files = {eq_by_dropped}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, metadata, + current_schema, file_io_); + + ASSERT_THAT(filter, IsOk()); + ASSERT_THAT(FieldIds(*filter.value()->RequiredSchema()), testing::ElementsAre(1, 7)); + const auto& dropped_field = filter.value()->RequiredSchema()->fields()[1]; + EXPECT_EQ(dropped_field.name(), "new_name"); + EXPECT_EQ(dropped_field.type()->type_id(), TypeId::kString); +} + +TEST_F(DeleteFilterTest, DroppedNestedFieldFiltersRowsWithSchemas) { + auto current_schema = std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", struct_({SchemaField::MakeOptional(5, "city", string())}))}, + /*schema_id=*/2); + auto historic_schema = std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional( + 4, "info", + struct_({SchemaField::MakeOptional(5, "city", string()), + SchemaField::MakeOptional(6, "state", string())}))}, + /*schema_id=*/1); + auto requested_schema = current_schema; + + EqualityDeleteWriterOptions options{ + .path = "eq-dropped-state-filter.parquet", + .schema = historic_schema, + .spec = partition_spec_, + .partition = PartitionValues{}, + .format = FileFormatType::kParquet, + .io = file_io_, + .equality_field_ids = {6}, + .properties = {{"write.parquet.compression-codec", "uncompressed"}}, + }; + ICEBERG_UNWRAP_OR_FAIL(auto writer, EqualityDeleteWriter::Make(options)); + ICEBERG_UNWRAP_OR_FAIL( + auto delete_batch, + MakeBatch(*historic_schema, + R"([{"id": 0, "info": {"city": "ignored", "state": "CA"}}])")); + ASSERT_THAT(writer->Write(&delete_batch.array), IsOk()); + ASSERT_THAT(writer->Close(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto eq_by_state_meta, writer->Metadata()); + auto eq_by_state = eq_by_state_meta.data_files[0]; + std::vector> delete_files = {eq_by_state}; + std::vector> schemas = {current_schema, historic_schema}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, current_schema, + requested_schema, file_io_, schemas); + ASSERT_THAT(filter, IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto data_batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([{"id": 1, "info": {"city": "SF", "state": "CA"}}, + {"id": 2, "info": {"city": "NYC", "state": "NY"}}, + {"id": 3, "info": null}])")); + + auto alive = filter.value()->ComputeAliveRows(data_batch.schema, data_batch.array); + + ASSERT_THAT(alive, IsOk()); + ExpectAliveRows(alive.value(), {1, 2}); +} + +TEST_F(DeleteFilterTest, DeletionVectorErrorPropagatesFromCompute) { + auto dv_file = std::make_shared(DataFile{ + .content = DataFile::Content::kPositionDeletes, + .file_path = "dv.puffin", + .file_format = FileFormatType::kPuffin, + }); + std::vector> delete_files = {dv_file}; + auto requested_schema = Project({1}); + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + + ASSERT_THAT(filter, IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto batch, + MakeBatch(*filter.value()->RequiredSchema(), R"([[1, 0]])")); + auto alive = filter.value()->ComputeAliveRows(batch.schema, batch.array); + ASSERT_THAT(alive, IsError(ErrorKind::kNotSupported)); +} + +TEST_F(DeleteFilterTest, EmptyBatchPropagatesDeleteLoadErrors) { + auto dv_file = std::make_shared(DataFile{ + .content = DataFile::Content::kPositionDeletes, + .file_path = "dv-empty.puffin", + .file_format = FileFormatType::kPuffin, + }); + std::vector> delete_files = {dv_file}; + auto requested_schema = Project({1}); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto batch, + MakeBatch(*filter.value()->RequiredSchema(), R"([])")); + + auto alive = filter.value()->ComputeAliveRows(batch.schema, batch.array); + + ASSERT_THAT(alive, IsError(ErrorKind::kNotSupported)); +} + +TEST_F(DeleteFilterTest, CounterAccumulatesAcrossBatches) { + ICEBERG_UNWRAP_OR_FAIL(auto pos_delete, + PositionDeleteFile("pos-multi-batch.parquet", {1})); + std::vector> delete_files = {pos_delete}; + auto requested_schema = Project({1}); + auto counter = std::make_shared(); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_, + /*need_row_pos_col=*/true, counter); + ASSERT_THAT(filter, IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto batch1, MakeBatch(*filter.value()->RequiredSchema(), + R"([[10, 0], [20, 1], [30, 2]])")); + ICEBERG_UNWRAP_OR_FAIL( + auto batch2, MakeBatch(*filter.value()->RequiredSchema(), R"([[40, 3], [50, 4]])")); + + ASSERT_THAT(filter.value()->ComputeAliveRows(batch1.schema, batch1.array), IsOk()); + ASSERT_THAT(filter.value()->ComputeAliveRows(batch2.schema, batch2.array), IsOk()); + EXPECT_EQ(counter->Get(), 1); +} + +enum class MakeErrorDeleteKind { + kNullDeleteFile, + kDataFile, + kEqualityDeleteWithEmptyIds, + kUnknownEqualityFieldId, +}; + +struct MakeErrorCase { + const char* name; + MakeErrorDeleteKind delete_kind; +}; + +class DeleteFilterMakeErrorTest : public DeleteFilterTest, + public testing::WithParamInterface {}; + +TEST_P(DeleteFilterMakeErrorTest, InvalidDeleteFilesError) { + const auto& test_case = GetParam(); + std::vector> delete_files; + switch (test_case.delete_kind) { + case MakeErrorDeleteKind::kNullDeleteFile: + delete_files.push_back(nullptr); + break; + case MakeErrorDeleteKind::kDataFile: + delete_files.push_back(std::make_shared(DataFile{ + .content = DataFile::Content::kData, + .file_path = "data.parquet", + .file_format = FileFormatType::kParquet, + })); + break; + case MakeErrorDeleteKind::kEqualityDeleteWithEmptyIds: + delete_files.push_back(std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-no-ids.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {}, + })); + break; + case MakeErrorDeleteKind::kUnknownEqualityFieldId: + delete_files.push_back(std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-unknown.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {999}, + })); + break; + } + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); +} + +INSTANTIATE_TEST_SUITE_P( + MakeErrors, DeleteFilterMakeErrorTest, + testing::Values( + MakeErrorCase{ + .name = "NullDeleteFile", + .delete_kind = MakeErrorDeleteKind::kNullDeleteFile, + }, + MakeErrorCase{ + .name = "DataFileAsDeleteFile", + .delete_kind = MakeErrorDeleteKind::kDataFile, + }, + MakeErrorCase{ + .name = "EqualityDeleteWithEmptyIds", + .delete_kind = MakeErrorDeleteKind::kEqualityDeleteWithEmptyIds, + }, + MakeErrorCase{ + .name = "UnknownEqualityFieldId", + .delete_kind = MakeErrorDeleteKind::kUnknownEqualityFieldId, + }), + ParamName); + +TEST_F(DeleteFilterTest, EqualityFieldNestedInListOrMapErrors) { + auto schema = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(4, "tags", + list(SchemaField::MakeRequired(5, "element", string()))), + SchemaField::MakeOptional(6, "attrs", + map(SchemaField::MakeRequired(7, "key", string()), + SchemaField::MakeOptional(8, "value", string())))}); + auto requested_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + + for (const auto& [field_id, nested_type] : + {std::pair{5, std::string_view("list")}, std::pair{8, std::string_view("map")}}) { + auto eq_delete = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-nested-container.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {field_id}, + }); + std::vector> delete_files = {eq_delete}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, schema, + requested_schema, file_io_); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(filter, + HasErrorMessage(std::format("must not be nested in {}", nested_type))); + } +} + +TEST_F(DeleteFilterTest, NullPosInBatchErrors) { + ICEBERG_UNWRAP_OR_FAIL(auto pos_delete, + PositionDeleteFile("pos-null-pos.parquet", {0})); + std::vector> delete_files = {pos_delete}; + auto requested_schema = Project({1}); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto batch, MakeBatch(*filter.value()->RequiredSchema(), + R"([[10, null], [20, null]])")); + + auto alive = filter.value()->ComputeAliveRows(batch.schema, batch.array); + + EXPECT_THAT(alive, IsError(ErrorKind::kInvalidArrowData)); +} + +TEST_F(DeleteFilterTest, ExpectedSchemaIsRequestedSchema) { + auto requested_schema = Project({1}); + auto eq_by_name = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-name.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {2}, + }); + std::vector> delete_files = {eq_by_name}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + EXPECT_EQ(filter.value()->ExpectedSchema(), requested_schema); + EXPECT_NE(filter.value()->RequiredSchema(), requested_schema); +} + +TEST_F(DeleteFilterTest, IncrementDeleteCountForwardsToCounter) { + std::vector> delete_files; + auto counter = std::make_shared(); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_, + /*need_row_pos_col=*/true, counter); + ASSERT_THAT(filter, IsOk()); + + filter.value()->IncrementDeleteCount(3); + filter.value()->IncrementDeleteCount(); + + EXPECT_EQ(counter->Get(), 4); +} + +TEST_F(DeleteFilterTest, DeletedRowPositionsNullWithNoPosDeletes) { + std::vector> delete_files; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto index = filter.value()->DeletedRowPositions(); + + ASSERT_THAT(index, IsOk()); + EXPECT_EQ(index.value(), nullptr); +} + +TEST_F(DeleteFilterTest, DeletedRowPositionsLazyLoads) { + ICEBERG_UNWRAP_OR_FAIL(auto pos_delete, + PositionDeleteFile("pos-index.parquet", {1, 3})); + std::vector> delete_files = {pos_delete}; + auto requested_schema = Project({1}); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto index = filter.value()->DeletedRowPositions(); + + ASSERT_THAT(index, IsOk()); + ASSERT_NE(index.value(), nullptr); + EXPECT_TRUE(index.value()->IsDeleted(1)); + EXPECT_TRUE(index.value()->IsDeleted(3)); + EXPECT_FALSE(index.value()->IsDeleted(0)); + EXPECT_FALSE(index.value()->IsDeleted(2)); +} + +TEST_F(DeleteFilterTest, EqDeletedRowFilterTrueWithNoEqDeletes) { + std::vector> delete_files; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto predicate_result = filter.value()->EqDeletedRowFilter(); + + ASSERT_THAT(predicate_result, IsOk()); + ASSERT_TRUE(static_cast(predicate_result.value())); + + ICEBERG_UNWRAP_OR_FAIL(auto batch, MakeBatch(*filter.value()->RequiredSchema(), + R"([[1, "Alice", "blue"]])")); + ICEBERG_UNWRAP_OR_FAIL(auto row, ArrowArrayStructLike::Make(batch.schema, batch.array)); + ICEBERG_UNWRAP_OR_FAIL(auto alive, predicate_result.value()(*row)); + EXPECT_TRUE(alive); +} + +TEST_F(DeleteFilterTest, EqDeletedRowFilterReturnsTrueForAliveRows) { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile("eq-filter.parquet", R"([[0, "Bob", "unused"]])", {2})); + std::vector> delete_files = {eq_by_name}; + auto requested_schema = Project({1}); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto predicate_result = filter.value()->EqDeletedRowFilter(); + ASSERT_THAT(predicate_result, IsOk()); + auto& predicate = predicate_result.value(); + ASSERT_TRUE(static_cast(predicate)); + + ICEBERG_UNWRAP_OR_FAIL(auto batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([[1, "Alice"], [2, "Bob"], [3, "Carol"]])")); + ICEBERG_UNWRAP_OR_FAIL(auto row, ArrowArrayStructLike::Make(batch.schema, batch.array)); + + ICEBERG_UNWRAP_OR_FAIL(auto alice_alive, predicate(*row)); + EXPECT_TRUE(alice_alive); + + ASSERT_THAT(row->Reset(1), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto bob_alive, predicate(*row)); + EXPECT_FALSE(bob_alive); + + ASSERT_THAT(row->Reset(2), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto carol_alive, predicate(*row)); + EXPECT_TRUE(carol_alive); +} + +TEST_F(DeleteFilterTest, EqDeletedRowFilterIsCached) { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile("eq-cache.parquet", R"([[0, "Bob", "unused"]])", {2})); + std::vector> delete_files = {eq_by_name}; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto result1 = filter.value()->EqDeletedRowFilter(); + auto result2 = filter.value()->EqDeletedRowFilter(); + ASSERT_THAT(result1, IsOk()); + ASSERT_THAT(result2, IsOk()); + EXPECT_TRUE(static_cast(result1.value())); + EXPECT_TRUE(static_cast(result2.value())); +} + +TEST_F(DeleteFilterTest, FindEqDeleteRowsTrueForDeleted) { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile("eq-find.parquet", R"([[0, "Bob", "unused"]])", {2})); + std::vector> delete_files = {eq_by_name}; + auto requested_schema = Project({1}); + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + requested_schema, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto predicate_result = filter.value()->FindEqualityDeleteRows(); + ASSERT_THAT(predicate_result, IsOk()); + auto& predicate = predicate_result.value(); + ASSERT_TRUE(static_cast(predicate)); + + ICEBERG_UNWRAP_OR_FAIL(auto batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([[1, "Alice"], [2, "Bob"], [3, "Carol"]])")); + ICEBERG_UNWRAP_OR_FAIL(auto row, ArrowArrayStructLike::Make(batch.schema, batch.array)); + + ICEBERG_UNWRAP_OR_FAIL(auto alice_deleted, predicate(*row)); + EXPECT_FALSE(alice_deleted); + + ASSERT_THAT(row->Reset(1), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto bob_deleted, predicate(*row)); + EXPECT_TRUE(bob_deleted); +} + +TEST_F(DeleteFilterTest, FindEqDeleteRowsFalseWithNoEqDeletes) { + std::vector> delete_files; + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, table_schema_, + table_schema_, file_io_); + ASSERT_THAT(filter, IsOk()); + + auto predicate_result = filter.value()->FindEqualityDeleteRows(); + + ASSERT_THAT(predicate_result, IsOk()); + ASSERT_TRUE(static_cast(predicate_result.value())); + + ICEBERG_UNWRAP_OR_FAIL(auto batch, MakeBatch(*filter.value()->RequiredSchema(), + R"([[1, "Alice", "blue"]])")); + ICEBERG_UNWRAP_OR_FAIL(auto row, ArrowArrayStructLike::Make(batch.schema, batch.array)); + ICEBERG_UNWRAP_OR_FAIL(auto deleted, predicate_result.value()(*row)); + EXPECT_FALSE(deleted); +} + +TEST_F(DeleteFilterTest, ExplicitFieldLookupFiltersRows) { + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile("eq-lookup.parquet", R"([[0, "Bob", "unused"]])", {2})); + std::vector> delete_files = {eq_by_name}; + auto requested_schema = Project({1}); + + ICEBERG_UNWRAP_OR_FAIL(auto base_lookup, DeleteFilter::MakeFieldLookup(table_schema_)); + DeleteFilter::FieldLookup custom_lookup = + [base_lookup = std::move(base_lookup)]( + int32_t field_id) -> Result> { + if (field_id == 2) { + return base_lookup(field_id); + } + return std::nullopt; + }; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, requested_schema, + file_io_, std::move(custom_lookup)); + + ASSERT_THAT(filter, IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto batch, + MakeBatch(*filter.value()->RequiredSchema(), + R"([[1, "Alice"], [2, "Bob"], [3, "Carol"]])")); + + auto alive = filter.value()->ComputeAliveRows(batch.schema, batch.array); + + ASSERT_THAT(alive, IsOk()); + ExpectAliveRows(alive.value(), {0, 2}); +} + +TEST_F(DeleteFilterTest, ExplicitFieldLookupNulloptErrors) { + // A lookup that returns nullopt for the equality field must produce an error + // at Make() time (during ComputeRequiredSchema), not silently skip the field. + auto eq_by_name = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-missing.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {2}, + }); + std::vector> delete_files = {eq_by_name}; + auto requested_schema = Project({1}); + + DeleteFilter::FieldLookup empty_lookup = + [](int32_t) -> Result> { + return std::nullopt; + }; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, requested_schema, + file_io_, std::move(empty_lookup)); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(DeleteFilterTest, ExplicitFieldLookupRejectsListOrMapProjection) { + auto eq_by_element = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-element.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {5}, + }); + std::vector> delete_files = {eq_by_element}; + auto requested_schema = Project({1}); + + DeleteFilter::FieldLookup list_lookup = + [](int32_t field_id) -> Result> { + auto element = SchemaField::MakeRequired(5, "element", string()); + return DeleteFilter::FieldLookupResult{ + .field = element, + .projection_field = SchemaField::MakeOptional(4, "tags", list(element)), + }; + }; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, requested_schema, + file_io_, std::move(list_lookup)); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(filter, HasErrorMessage("must not be nested in list")); +} + +TEST_F(DeleteFilterTest, ExplicitFieldLookupSkipsExistingFields) { + // When the equality field is already in requested_schema, the custom lookup + // must NOT be called + ICEBERG_UNWRAP_OR_FAIL( + auto eq_by_name, + EqualityDeleteFile("eq-already-present.parquet", R"([[0, "Bob", "unused"]])", {2})); + std::vector> delete_files = {eq_by_name}; + auto requested_schema = Project({1, 2}); + + bool lookup_called = false; + DeleteFilter::FieldLookup tracking_lookup = + [&lookup_called]( + int32_t) -> Result> { + lookup_called = true; + return std::nullopt; // would fail if called + }; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, requested_schema, + file_io_, std::move(tracking_lookup)); + + ASSERT_THAT(filter, IsOk()); + EXPECT_FALSE(lookup_called); +} + +TEST_F(DeleteFilterTest, SchemasLookupDeduplicatesCurrentSchemaId) { + auto current_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}, + /*schema_id=*/2); + auto same_id_historic_schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32()), + SchemaField::MakeOptional(7, "not_historic", string())}, + /*schema_id=*/2); + auto eq_by_dropped = std::make_shared(DataFile{ + .content = DataFile::Content::kEqualityDeletes, + .file_path = "eq-dropped.parquet", + .file_format = FileFormatType::kParquet, + .equality_ids = {7}, + }); + std::vector> delete_files = {eq_by_dropped}; + std::vector> schemas = {current_schema, + same_id_historic_schema}; + + auto filter = DeleteFilter::Make(std::string(kDataPath), delete_files, current_schema, + current_schema, file_io_, schemas); + + EXPECT_THAT(filter, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(filter, HasErrorMessage("Cannot find equality delete field id 7")); +} + +} // namespace iceberg diff --git a/src/iceberg/test/expire_snapshots_test.cc b/src/iceberg/test/expire_snapshots_test.cc index 4dcc72d6c..3a99b0009 100644 --- a/src/iceberg/test/expire_snapshots_test.cc +++ b/src/iceberg/test/expire_snapshots_test.cc @@ -28,6 +28,9 @@ #include "iceberg/avro/avro_register.h" #include "iceberg/manifest/manifest_entry.h" #include "iceberg/manifest/manifest_writer.h" +#include "iceberg/partition_spec.h" +#include "iceberg/schema.h" +#include "iceberg/snapshot.h" #include "iceberg/statistics_file.h" #include "iceberg/table_metadata.h" #include "iceberg/test/matchers.h" @@ -135,6 +138,13 @@ class ExpireSnapshotsCleanupTest : public UpdateTestBase { return manifest_result.value(); } + ManifestFile AssignManifestSequenceNumber(ManifestFile manifest, + int64_t sequence_number) const { + manifest.sequence_number = sequence_number; + manifest.min_sequence_number = sequence_number; + return manifest; + } + ManifestFile WriteDeleteManifest(const std::string& path, int64_t snapshot_id, std::vector entries) { auto writer_result = ManifestWriter::MakeWriter( @@ -227,6 +237,15 @@ TEST_F(ExpireSnapshotsTest, ExpireById) { EXPECT_EQ(result.snapshot_ids_to_remove.at(0), 3051729675574597004); } +TEST_F(ExpireSnapshotsTest, ExpireByIdOverridesRetainLast) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->RetainLast(2); + update->ExpireSnapshotId(3051729675574597004); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + EXPECT_THAT(result.snapshot_ids_to_remove, testing::ElementsAre(3051729675574597004)); +} + TEST_F(ExpireSnapshotsTest, ExpireOlderThan) { struct TestCase { int64_t expire_older_than; @@ -243,6 +262,30 @@ TEST_F(ExpireSnapshotsTest, ExpireOlderThan) { } } +TEST_F(ExpireSnapshotsCleanupTest, RetainsUnreferencedSnapshotAtExpireThreshold) { + const int64_t unreferenced_snapshot_id = 4055729675574597004; + const int64_t expire_at_ms = 1515100955770; + + auto metadata = ReloadMetadata(); + metadata->snapshots.push_back(std::make_shared(Snapshot{ + .snapshot_id = unreferenced_snapshot_id, + .parent_snapshot_id = std::nullopt, + .sequence_number = 2, + .timestamp_ms = TimePointMsFromUnixMs(expire_at_ms), + .manifest_list = table_location_ + "/metadata/unreferenced.avro", + .summary = {{SnapshotSummaryFields::kOperation, "append"}}, + .schema_id = metadata->current_schema_id, + })); + RewriteTable(std::move(metadata)); + + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->ExpireOlderThan(expire_at_ms); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + EXPECT_THAT(result.snapshot_ids_to_remove, + testing::Not(testing::Contains(unreferenced_snapshot_id))); +} + TEST_F(ExpireSnapshotsTest, FinalizeRequiresCommittedMetadata) { std::vector deleted_files; ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); @@ -350,6 +393,8 @@ TEST_F(ExpireSnapshotsCleanupTest, IgnoresExpiredDeleteManifestReadFailures) { std::vector deleted_files; ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + // Force the reachable path. + update->ExpireSnapshotId(kExpiredSnapshotId); update->DeleteWith( [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); @@ -388,6 +433,7 @@ TEST_F(ExpireSnapshotsCleanupTest, DeletesExpiredFiles) { std::vector deleted_files; ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->ExpireSnapshotId(kExpiredSnapshotId); update->DeleteWith( [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); @@ -573,4 +619,216 @@ TEST_F(ExpireSnapshotsCleanupTest, KeepsReusedPartitionStats) { EXPECT_THAT(deleted_files, testing::Not(testing::Contains(reused_statistics_path))); } +TEST_F(ExpireSnapshotsCleanupTest, IncrementalDispatchPreservesAncestorAddedFiles) { + const auto expired_data_file_path = table_location_ + "/data/expired-data.parquet"; + const auto expired_data_manifest_path = table_location_ + "/metadata/expired-data.avro"; + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-manifest-list.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-manifest-list.avro"; + + auto expired_data_manifest = WriteDataManifest( + expired_data_manifest_path, kExpiredSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kExpiredSnapshotId, kExpiredSequenceNumber, + MakeDataFile(expired_data_file_path))}); + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, + {expired_data_manifest}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {}); + RewriteTableWithManifestLists(expired_manifest_list_path, current_manifest_list_path); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_THAT(deleted_files, testing::Contains(expired_data_manifest_path)); + EXPECT_THAT(deleted_files, testing::Contains(expired_manifest_list_path)); + EXPECT_THAT(deleted_files, testing::Not(testing::Contains(expired_data_file_path))); +} + +TEST_F(ExpireSnapshotsCleanupTest, IncrementalDeletesExpiredDeletedEntries) { + const auto deleted_data_file_path = + table_location_ + "/data/deleted-by-expired.parquet"; + const auto delete_manifest_path = + table_location_ + "/metadata/expired-delete-entry.avro"; + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-deleted-entry-ml.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-deleted-entry-ml.avro"; + + auto delete_manifest = WriteDataManifest( + delete_manifest_path, kExpiredSnapshotId, + {MakeEntry(ManifestStatus::kDeleted, kExpiredSnapshotId, kExpiredSequenceNumber, + MakeDataFile(deleted_data_file_path))}); + delete_manifest = + AssignManifestSequenceNumber(std::move(delete_manifest), kExpiredSequenceNumber); + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, {delete_manifest}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {delete_manifest}); + RewriteTableWithManifestLists(expired_manifest_list_path, current_manifest_list_path); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_THAT(deleted_files, testing::Contains(deleted_data_file_path)); + EXPECT_THAT(deleted_files, testing::Contains(expired_manifest_list_path)); + EXPECT_THAT(deleted_files, testing::Not(testing::Contains(delete_manifest_path))); +} + +TEST_F(ExpireSnapshotsCleanupTest, ReachableDispatchDeletesUnreachableData) { + const auto expired_data_file_path = table_location_ + "/data/expired-data.parquet"; + const auto expired_data_manifest_path = table_location_ + "/metadata/expired-data.avro"; + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-manifest-list.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-manifest-list.avro"; + + auto expired_data_manifest = WriteDataManifest( + expired_data_manifest_path, kExpiredSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kExpiredSnapshotId, kExpiredSequenceNumber, + MakeDataFile(expired_data_file_path))}); + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, + {expired_data_manifest}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {}); + RewriteTableWithManifestLists(expired_manifest_list_path, current_manifest_list_path); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->ExpireSnapshotId(kExpiredSnapshotId); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_THAT(deleted_files, testing::UnorderedElementsAre(expired_data_file_path, + expired_data_manifest_path, + expired_manifest_list_path)); +} + +TEST_F(ExpireSnapshotsCleanupTest, IncrementalSkipsCherryPickedSnapshotCleanup) { + const auto picked_data_file_path = table_location_ + "/data/picked-data.parquet"; + const auto picked_manifest_path = table_location_ + "/metadata/picked-data.avro"; + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-picked-ml.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-picked-ml.avro"; + + auto picked_manifest = WriteDataManifest( + picked_manifest_path, kExpiredSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kExpiredSnapshotId, kExpiredSequenceNumber, + MakeDataFile(picked_data_file_path))}); + picked_manifest = + AssignManifestSequenceNumber(std::move(picked_manifest), kExpiredSequenceNumber); + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, {picked_manifest}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {picked_manifest}); + + auto metadata = ReloadMetadata(); + ASSERT_EQ(metadata->snapshots.size(), 2); + metadata->snapshots.at(0)->manifest_list = expired_manifest_list_path; + metadata->snapshots.at(1)->manifest_list = current_manifest_list_path; + metadata->snapshots.at(1)->summary[SnapshotSummaryFields::kSourceSnapshotId] = + std::to_string(kExpiredSnapshotId); + RewriteTable(std::move(metadata)); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_TRUE(deleted_files.empty()); + auto committed_metadata = ReloadMetadata(); + EXPECT_EQ(committed_metadata->snapshots.size(), 1); + EXPECT_EQ(committed_metadata->snapshots.at(0)->snapshot_id, kCurrentSnapshotId); +} + +TEST_F(ExpireSnapshotsCleanupTest, ReachableCleanupFailsClosedOnUnbindableExpiredSpec) { + const auto expired_data_file_path = table_location_ + "/data/expired-data.parquet"; + const auto expired_data_manifest_path = table_location_ + "/metadata/expired-data.avro"; + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-manifest-list.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-manifest-list.avro"; + + auto expired_data_manifest = WriteDataManifest( + expired_data_manifest_path, kExpiredSnapshotId, + {MakeEntry(ManifestStatus::kAdded, kExpiredSnapshotId, kExpiredSequenceNumber, + MakeDataFile(expired_data_file_path))}); + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, + {expired_data_manifest}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {}); + + auto metadata = ReloadMetadata(); + ASSERT_EQ(metadata->snapshots.size(), 2); + metadata->snapshots.at(0)->manifest_list = expired_manifest_list_path; + metadata->snapshots.at(1)->manifest_list = current_manifest_list_path; + ICEBERG_UNWRAP_OR_FAIL(auto retained_spec, PartitionSpec::Make(/*spec_id=*/1, {})); + metadata->partition_specs.push_back( + std::shared_ptr(std::move(retained_spec))); + metadata->default_spec_id = 1; + ICEBERG_UNWRAP_OR_FAIL( + auto retained_schema, + Schema::Make(std::vector{SchemaField::MakeRequired(2, "y", int64()), + SchemaField::MakeRequired(3, "z", int64())}, + /*schema_id=*/2, std::vector{})); + metadata->schemas.push_back(std::shared_ptr(std::move(retained_schema))); + metadata->current_schema_id = 2; + metadata->snapshots.at(1)->schema_id = 2; + RewriteTable(std::move(metadata)); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->ExpireSnapshotId(kExpiredSnapshotId); + update->CleanExpiredMetadata(true); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_THAT(deleted_files, testing::UnorderedElementsAre(expired_data_manifest_path, + expired_manifest_list_path)); + EXPECT_THAT(deleted_files, testing::Not(testing::Contains(expired_data_file_path))); +} + +TEST_F(ExpireSnapshotsCleanupTest, CommitIgnoresMalformedSourceSnapshotIdCleanup) { + const auto expired_manifest_list_path = + table_location_ + "/metadata/expired-malformed-ml.avro"; + const auto current_manifest_list_path = + table_location_ + "/metadata/current-malformed-ml.avro"; + WriteManifestList(expired_manifest_list_path, kExpiredSnapshotId, + /*parent_snapshot_id=*/0, kExpiredSequenceNumber, {}); + WriteManifestList(current_manifest_list_path, kCurrentSnapshotId, kExpiredSnapshotId, + kCurrentSequenceNumber, {}); + + auto metadata = ReloadMetadata(); + ASSERT_EQ(metadata->snapshots.size(), 2); + metadata->snapshots.at(0)->manifest_list = expired_manifest_list_path; + metadata->snapshots.at(1)->manifest_list = current_manifest_list_path; + metadata->snapshots.at(1)->summary[SnapshotSummaryFields::kSourceSnapshotId] = + "not-a-number"; + RewriteTable(std::move(metadata)); + + std::vector deleted_files; + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewExpireSnapshots()); + update->DeleteWith( + [&deleted_files](const std::string& path) { deleted_files.push_back(path); }); + + EXPECT_THAT(update->Commit(), IsOk()); + EXPECT_TRUE(deleted_files.empty()); + auto committed_metadata = ReloadMetadata(); + EXPECT_EQ(committed_metadata->snapshots.size(), 1); + EXPECT_EQ(committed_metadata->snapshots.at(0)->snapshot_id, kCurrentSnapshotId); +} + } // namespace iceberg diff --git a/src/iceberg/test/file_io_test.cc b/src/iceberg/test/file_io_test.cc new file mode 100644 index 000000000..0908572f9 --- /dev/null +++ b/src/iceberg/test/file_io_test.cc @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/file_io.h" + +#include +#include +#include + +#include + +#include "iceberg/test/matchers.h" + +namespace iceberg { +namespace { + +class RecordingFileIO : public FileIO { + public: + explicit RecordingFileIO(std::string failure_path = "") + : failure_path_(std::move(failure_path)) {} + + Status DeleteFile(const std::string& file_location) override { + deleted_paths.push_back(file_location); + if (file_location == failure_path_) { + return IOError("failed to delete {}", file_location); + } + return {}; + } + + std::vector deleted_paths; + + private: + std::string failure_path_; +}; + +} // namespace + +TEST(FileIOTest, DeleteFilesFallsBackToDeleteFileForEachPath) { + RecordingFileIO file_io; + std::vector paths = {"file-a.avro", "file-b.avro"}; + + EXPECT_THAT(file_io.DeleteFiles(paths), IsOk()); + EXPECT_THAT(file_io.deleted_paths, + ::testing::ElementsAre("file-a.avro", "file-b.avro")); +} + +TEST(FileIOTest, DeleteFilesReturnsFirstDeleteFileError) { + RecordingFileIO file_io("file-b.avro"); + std::vector paths = {"file-a.avro", "file-b.avro", "file-c.avro"}; + + auto status = file_io.DeleteFiles(paths); + + EXPECT_THAT(status, IsError(ErrorKind::kIOError)); + EXPECT_THAT(status, HasErrorMessage("failed to delete file-b.avro")); + EXPECT_THAT(file_io.deleted_paths, + ::testing::ElementsAre("file-a.avro", "file-b.avro")); +} + +} // namespace iceberg diff --git a/src/iceberg/test/manifest_filter_manager_test.cc b/src/iceberg/test/manifest_filter_manager_test.cc new file mode 100644 index 000000000..8fc1c61ed --- /dev/null +++ b/src/iceberg/test/manifest_filter_manager_test.cc @@ -0,0 +1,562 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/manifest/manifest_filter_manager.h" + +#include +#include +#include + +#include +#include + +#include "iceberg/avro/avro_register.h" +#include "iceberg/expression/expression.h" +#include "iceberg/expression/expressions.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/partition_spec.h" +#include "iceberg/result.h" +#include "iceberg/row/partition_values.h" +#include "iceberg/schema.h" +#include "iceberg/table.h" +#include "iceberg/table_metadata.h" +#include "iceberg/test/matchers.h" +#include "iceberg/test/update_test_base.h" +#include "iceberg/update/fast_append.h" +#include "iceberg/util/data_file_set.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +class ManifestFilterManagerTest : public MinimalUpdateTestBase { + protected: + static void SetUpTestSuite() { avro::RegisterAll(); } + + void SetUp() override { + MinimalUpdateTestBase::SetUp(); + + ICEBERG_UNWRAP_OR_FAIL(spec_, table_->spec()); + ICEBERG_UNWRAP_OR_FAIL(schema_, table_->schema()); + + // Two files in different partitions (identity(x)) + file_a_ = MakeDataFile("/data/file_a.parquet", /*partition_x=*/1L); + file_b_ = MakeDataFile("/data/file_b.parquet", /*partition_x=*/2L); + } + + std::shared_ptr MakeDataFile(const std::string& path, int64_t partition_x) { + auto f = std::make_shared(); + f->content = DataFile::Content::kData; + f->file_path = table_location_ + path; + f->file_format = FileFormatType::kParquet; + f->partition = PartitionValues(std::vector{Literal::Long(partition_x)}); + f->file_size_in_bytes = 1024; + f->record_count = 100; + f->partition_spec_id = spec_->spec_id(); + return f; + } + + // Append files, commit, refresh, and return the current snapshot. + Result> CommitFiles( + std::vector> files) { + ICEBERG_ASSIGN_OR_RAISE(auto fa, table_->NewFastAppend()); + for (const auto& f : files) fa->AppendFile(f); + ICEBERG_RETURN_UNEXPECTED(fa->Commit()); + ICEBERG_RETURN_UNEXPECTED(table_->Refresh()); + return table_->current_snapshot(); + } + + ManifestWriterFactory MakeWriterFactory(const TableMetadata& metadata) { + auto fv = metadata.format_version; + return [this, fv, &metadata](int32_t spec_id, ManifestContent content) mutable + -> Result> { + ICEBERG_ASSIGN_OR_RAISE(auto spec, metadata.PartitionSpecById(spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + auto path = + std::format("{}/metadata/flt-{}.avro", table_location_, manifest_counter_++); + return ManifestWriter::MakeWriter(fv, kTestSnapshotId, path, file_io_, spec, schema, + content); + }; + } + + ManifestFilterManager::PartitionSpecsById SpecsById(const TableMetadata& metadata) { + ManifestFilterManager::PartitionSpecsById specs_by_id; + for (const auto& spec : metadata.partition_specs) { + specs_by_id.emplace(spec->spec_id(), spec); + } + return specs_by_id; + } + + // Read all entries from a list of ManifestFiles. + Result> ReadAllEntries( + const std::vector& manifests, const TableMetadata& metadata) { + std::vector result; + for (const auto& m : manifests) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, metadata.PartitionSpecById(m.partition_spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(m, file_io_, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + result.insert(result.end(), entries.begin(), entries.end()); + } + return result; + } + + static constexpr int64_t kTestSnapshotId = 55555L; + int manifest_counter_ = 0; + std::shared_ptr spec_; + std::shared_ptr schema_; + std::shared_ptr file_a_; + std::shared_ptr file_b_; +}; + +TEST_F(ManifestFilterManagerTest, NullSnapshotReturnsEmpty) { + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, nullptr, factory)); + EXPECT_TRUE(result.empty()); +} + +TEST_F(ManifestFilterManagerTest, ContainsDeletesReturnsCorrectState) { + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + EXPECT_FALSE(mgr.ContainsDeletes()); + mgr.DeleteFile("/some/path.parquet"); + EXPECT_TRUE(mgr.ContainsDeletes()); +} + +TEST_F(ManifestFilterManagerTest, DeleteByRowFilterRejectsNull) { + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + EXPECT_THAT(mgr.DeleteByRowFilter(nullptr), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(ManifestFilterManagerTest, DeleteFileObjectRejectsNull) { + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + std::shared_ptr null_file; + EXPECT_THAT(mgr.DeleteFile(null_file), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(ManifestFilterManagerTest, NoConditionsReturnsManifestsUnchanged) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + // Load original manifests so we can compare paths + ICEBERG_UNWRAP_OR_FAIL(auto list_reader, + ManifestListReader::Make(snap->manifest_list, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto orig_manifests, list_reader->Files()); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ASSERT_EQ(result.size(), orig_manifests.size()); + for (size_t i = 0; i < result.size(); ++i) { + // No rewrite → same manifest path + EXPECT_EQ(result[i].manifest_path, orig_manifests[i].manifest_path); + } +} + +TEST_F(ManifestFilterManagerTest, DeleteFileByPath) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DeleteFile(file_a_->file_path); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + int deleted_count = 0; + int live_count = 0; + for (const auto& e : entries) { + if (e.status == ManifestStatus::kDeleted) { + ++deleted_count; + ASSERT_NE(e.data_file, nullptr); + EXPECT_EQ(e.data_file->file_path, file_a_->file_path); + } else { + ++live_count; + } + } + EXPECT_EQ(deleted_count, 1); + EXPECT_EQ(live_count, 1); +} + +TEST_F(ManifestFilterManagerTest, ExplicitContextFilterManifestsDeletesByPath) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ICEBERG_UNWRAP_OR_FAIL(auto list_reader, + ManifestListReader::Make(snap->manifest_list, file_io_)); + ICEBERG_UNWRAP_OR_FAIL(auto manifest_files, list_reader->Files()); + std::vector manifests; + manifests.reserve(manifest_files.size()); + for (const auto& manifest : manifest_files) { + manifests.push_back(&manifest); + } + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DeleteFile(file_a_->file_path); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(schema_, SpecsById(*metadata), + manifests, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + int deleted_count = 0; + for (const auto& entry : entries) { + if (entry.status == ManifestStatus::kDeleted) { + ++deleted_count; + ASSERT_NE(entry.data_file, nullptr); + EXPECT_EQ(entry.data_file->file_path, file_a_->file_path); + } + } + EXPECT_EQ(deleted_count, 1); +} + +TEST_F(ManifestFilterManagerTest, RowFilterAlwaysTrueDeletesAll) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::AlwaysTrue()), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + for (const auto& e : entries) { + EXPECT_EQ(e.status, ManifestStatus::kDeleted) << "Expected all entries to be DELETED"; + } +} + +TEST_F(ManifestFilterManagerTest, RowFilterAlwaysFalseDeletesNone) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::AlwaysFalse()), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + for (const auto& e : entries) { + // AlwaysFalse means nothing can match → entries remain ADDED or EXISTING + EXPECT_NE(e.status, ManifestStatus::kDeleted) << "Expected no entries to be DELETED"; + } +} + +TEST_F(ManifestFilterManagerTest, RowFilterUsesPartitionResiduals) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.CaseSensitive(false); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::Equal("X", Literal::Long(1L))), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + int deleted_count = 0; + int live_count = 0; + for (const auto& e : entries) { + ASSERT_NE(e.data_file, nullptr); + if (e.status == ManifestStatus::kDeleted) { + ++deleted_count; + EXPECT_EQ(e.data_file->file_path, file_a_->file_path); + } else { + ++live_count; + EXPECT_EQ(e.data_file->file_path, file_b_->file_path); + } + } + + EXPECT_EQ(deleted_count, 1); + EXPECT_EQ(live_count, 1); + ASSERT_EQ(mgr.FilesToBeDeleted().size(), 1U); + EXPECT_EQ(mgr.FilesToBeDeleted().begin()->get()->file_path, file_a_->file_path); +} + +TEST_F(ManifestFilterManagerTest, DropPartition) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + // Drop partition of file_a (partition_x = 1) + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DropPartition(spec_->spec_id(), + PartitionValues(std::vector{Literal::Long(1L)})); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + int deleted_count = 0; + for (const auto& e : entries) { + if (e.status == ManifestStatus::kDeleted) { + ++deleted_count; + ASSERT_TRUE(e.data_file != nullptr); + EXPECT_EQ(e.data_file->file_path, file_a_->file_path); + } + } + EXPECT_EQ(deleted_count, 1); +} + +TEST_F(ManifestFilterManagerTest, FailMissingDeletePathsReturnsError) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DeleteFile("/does/not/exist.parquet"); + mgr.FailMissingDeletePaths(); + + auto result = mgr.FilterManifests(*metadata, snap, factory); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(ManifestFilterManagerTest, FailAnyDeleteReportsPartitionPath) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DeleteFile(file_a_->file_path); + mgr.FailAnyDelete(); + + auto result = mgr.FilterManifests(*metadata, snap, factory); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, HasErrorMessage("x=1")); +} + +TEST_F(ManifestFilterManagerTest, MultipleConditionsOrCombined) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_, file_b_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + // Both files should be deleted: file_a by path, file_b by AlwaysTrue expression + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + mgr.DeleteFile(file_a_->file_path); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::AlwaysTrue()), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + for (const auto& e : entries) { + EXPECT_EQ(e.status, ManifestStatus::kDeleted); + } +} + +TEST_F(ManifestFilterManagerTest, MultipleRowFiltersUseCombinedExpression) { + ICEBERG_UNWRAP_OR_FAIL(auto snap, CommitFiles({file_a_})); + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + ManifestFilterManager mgr(ManifestContent::kData, file_io_); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::Equal("y", Literal::Long(7L))), IsOk()); + ASSERT_THAT(mgr.DeleteByRowFilter(Expressions::Equal("x", Literal::Long(1L))), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, mgr.FilterManifests(*metadata, snap, factory)); + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + + ASSERT_EQ(entries.size(), 1U); + EXPECT_EQ(entries[0].status, ManifestStatus::kDeleted); +} + +// Helper: write one or more delete-file entries to a new manifest. +// Each entry is (DataFile, data_sequence_number). +using DeleteManifestEntry = std::pair, int64_t>; +static Result WriteDeleteManifest( + const std::vector& files, std::shared_ptr file_io, + const TableMetadata& metadata, const std::string& path) { + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + int32_t spec_id = files[0].first->partition_spec_id.value_or(0); + ICEBERG_ASSIGN_OR_RAISE(auto spec, metadata.PartitionSpecById(spec_id)); + ICEBERG_ASSIGN_OR_RAISE( + auto writer, + ManifestWriter::MakeWriter(metadata.format_version, /*snapshot_id=*/1L, path, + file_io, spec, schema, ManifestContent::kDeletes)); + for (auto& [file, seq] : files) { + ManifestEntry entry; + entry.status = ManifestStatus::kAdded; + entry.snapshot_id = 1L; + entry.sequence_number = seq; + entry.data_file = file; + ICEBERG_RETURN_UNEXPECTED(writer->WriteAddedEntry(entry)); + } + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + return writer->ToManifestFile(); +} + +// Convenience overload for a single entry. +static Result WriteDeleteManifest(std::shared_ptr delete_file, + int64_t data_sequence_number, + std::shared_ptr file_io, + const TableMetadata& metadata, + const std::string& path) { + return WriteDeleteManifest({{delete_file, data_sequence_number}}, file_io, metadata, + path); +} + +TEST_F(ManifestFilterManagerTest, DropDeleteFilesOlderThan) { + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + // Create a position-delete file with data_sequence_number = 2 (below threshold 5). + auto del_file = std::make_shared(); + del_file->content = DataFile::Content::kPositionDeletes; + del_file->file_path = table_location_ + "/delete/del_old.parquet"; + del_file->file_format = FileFormatType::kParquet; + del_file->partition = PartitionValues(std::vector{Literal::Long(1L)}); + del_file->file_size_in_bytes = 512; + del_file->record_count = 10; + del_file->partition_spec_id = spec_->spec_id(); + + auto manifest_path = std::format("{}/metadata/del-manifest-{}.avro", table_location_, + manifest_counter_++); + ICEBERG_UNWRAP_OR_FAIL( + auto del_manifest, + WriteDeleteManifest(del_file, /*data_seq=*/2L, file_io_, *metadata, manifest_path)); + + ManifestFilterManager mgr(ManifestContent::kDeletes, file_io_); + // Drop delete files older than sequence number 5: entry (seq=2) should be dropped. + mgr.DropDeleteFilesOlderThan(5); + + std::vector manifests{&del_manifest}; + auto specs = SpecsById(*metadata); + ICEBERG_UNWRAP_OR_FAIL(auto schema, metadata->Schema()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.FilterManifests(schema, specs, manifests, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + ASSERT_EQ(entries.size(), 1U); + EXPECT_EQ(entries[0].status, ManifestStatus::kDeleted); +} + +TEST_F(ManifestFilterManagerTest, DropDeleteFilesOlderThan_KeepsNewerEntries) { + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + // Two entries in the same manifest: old (seq=2, below threshold) and new (seq=10, + // above). + auto make_del_file = [&](const std::string& path) { + auto f = std::make_shared(); + f->content = DataFile::Content::kPositionDeletes; + f->file_path = path; + f->file_format = FileFormatType::kParquet; + f->partition = PartitionValues(std::vector{Literal::Long(1L)}); + f->file_size_in_bytes = 512; + f->record_count = 10; + f->partition_spec_id = spec_->spec_id(); + return f; + }; + auto old_file = make_del_file(table_location_ + "/delete/del_old.parquet"); + auto new_file = make_del_file(table_location_ + "/delete/del_new.parquet"); + + auto manifest_path = std::format("{}/metadata/del-manifest-{}.avro", table_location_, + manifest_counter_++); + ICEBERG_UNWRAP_OR_FAIL(auto del_manifest, + WriteDeleteManifest({{old_file, 2L}, {new_file, 10L}}, file_io_, + *metadata, manifest_path)); + + ManifestFilterManager mgr(ManifestContent::kDeletes, file_io_); + // Threshold=5: old entry dropped, new entry survives as kExisting. + mgr.DropDeleteFilesOlderThan(5); + + std::vector manifests{&del_manifest}; + auto specs = SpecsById(*metadata); + ICEBERG_UNWRAP_OR_FAIL(auto schema, metadata->Schema()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.FilterManifests(schema, specs, manifests, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + ASSERT_EQ(entries.size(), 2U); + // The old entry should be dropped; the new entry should survive. + auto deleted = std::count_if( + entries.begin(), entries.end(), + [](const ManifestEntry& e) { return e.status == ManifestStatus::kDeleted; }); + auto existing = std::count_if( + entries.begin(), entries.end(), + [](const ManifestEntry& e) { return e.status == ManifestStatus::kExisting; }); + EXPECT_EQ(deleted, 1); + EXPECT_EQ(existing, 1); + // Verify which entry survived. + for (const auto& e : entries) { + if (e.status == ManifestStatus::kExisting) { + EXPECT_EQ(e.data_file->file_path, new_file->file_path); + } else { + EXPECT_EQ(e.data_file->file_path, old_file->file_path); + } + } +} + +TEST_F(ManifestFilterManagerTest, RemoveDanglingDeletesFor_FiltersDanglingDV) { + auto* metadata = table_->metadata().get(); + auto factory = MakeWriterFactory(*metadata); + + const std::string data_file_path = table_location_ + "/data/referenced.parquet"; + + // Create a DV (position-delete, puffin format) referencing the data file. + auto dv_file = std::make_shared(); + dv_file->content = DataFile::Content::kPositionDeletes; + dv_file->file_path = table_location_ + "/delete/dv.puffin"; + dv_file->file_format = FileFormatType::kPuffin; + dv_file->referenced_data_file = data_file_path; + dv_file->partition = PartitionValues(std::vector{Literal::Long(1L)}); + dv_file->file_size_in_bytes = 256; + dv_file->record_count = 5; + dv_file->partition_spec_id = spec_->spec_id(); + + auto manifest_path = std::format("{}/metadata/dv-manifest-{}.avro", table_location_, + manifest_counter_++); + ICEBERG_UNWRAP_OR_FAIL( + auto dv_manifest, + WriteDeleteManifest(dv_file, /*data_seq=*/3L, file_io_, *metadata, manifest_path)); + + // Register the referenced data file as deleted. + auto deleted_data_file = std::make_shared(); + deleted_data_file->content = DataFile::Content::kData; + deleted_data_file->file_path = data_file_path; + deleted_data_file->partition = PartitionValues(std::vector{Literal::Long(1L)}); + deleted_data_file->file_size_in_bytes = 1024; + deleted_data_file->record_count = 50; + deleted_data_file->partition_spec_id = spec_->spec_id(); + + DataFileSet deleted_files; + deleted_files.insert(deleted_data_file); + + ManifestFilterManager mgr(ManifestContent::kDeletes, file_io_); + mgr.RemoveDanglingDeletesFor(deleted_files); + + std::vector manifests{&dv_manifest}; + auto specs = SpecsById(*metadata); + ICEBERG_UNWRAP_OR_FAIL(auto schema, metadata->Schema()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.FilterManifests(schema, specs, manifests, factory)); + + ICEBERG_UNWRAP_OR_FAIL(auto entries, ReadAllEntries(result, *metadata)); + ASSERT_EQ(entries.size(), 1U); + EXPECT_EQ(entries[0].status, ManifestStatus::kDeleted); +} + +} // namespace iceberg diff --git a/src/iceberg/test/manifest_merge_manager_test.cc b/src/iceberg/test/manifest_merge_manager_test.cc new file mode 100644 index 000000000..b19eace86 --- /dev/null +++ b/src/iceberg/test/manifest_merge_manager_test.cc @@ -0,0 +1,355 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/manifest/manifest_merge_manager.h" + +#include +#include +#include +#include + +#include +#include + +#include "iceberg/arrow/arrow_io_util.h" +#include "iceberg/avro/avro_register.h" +#include "iceberg/file_format.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/partition_spec.h" +#include "iceberg/result.h" +#include "iceberg/row/partition_values.h" +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/sort_order.h" +#include "iceberg/table_metadata.h" +#include "iceberg/test/matchers.h" +#include "iceberg/transform.h" +#include "iceberg/type.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +namespace { + +constexpr int8_t kFormatVersion = 2; +constexpr int64_t kSnapshotId = 12345L; +constexpr int32_t kSpecId0 = 0; +constexpr int32_t kSpecId1 = 1; + +} // namespace + +class ManifestMergeManagerTest : public ::testing::Test { + protected: + static void SetUpTestSuite() { avro::RegisterAll(); } + + void SetUp() override { + file_io_ = arrow::MakeMockFileIO(); + + // Simple schema: one long column + schema_ = std::make_shared(std::vector{ + SchemaField::MakeRequired(1, "x", int64()), + }); + spec0_ = PartitionSpec::Make(kSpecId0, + {PartitionField(1, 1000, "x", Transform::Identity())}) + .value(); + spec1_ = PartitionSpec::Make( + kSpecId1, {PartitionField(1, 1001, "x_bucket", Transform::Bucket(8))}) + .value(); + + // Build minimal TableMetadata with both specs + auto builder = TableMetadataBuilder::BuildFromEmpty(kFormatVersion); + builder->SetCurrentSchema(schema_, schema_->HighestFieldId().value_or(0)); + builder->SetDefaultPartitionSpec(spec0_); + builder->AddPartitionSpec(spec1_); + builder->SetDefaultSortOrder(SortOrder::Unsorted()); + ICEBERG_UNWRAP_OR_FAIL(auto metadata, builder->Build()); + metadata_ = std::shared_ptr(std::move(metadata)); + } + + // Write a small manifest with N data files and return the ManifestFile descriptor. + Result WriteManifest(int32_t spec_id, int num_files, + int64_t file_size_override = 512, + ManifestContent content = ManifestContent::kData) { + auto path = std::format("manifest-{}.avro", manifest_counter_++); + auto spec = spec_id == kSpecId0 ? spec0_ : spec1_; + ICEBERG_ASSIGN_OR_RAISE(auto writer, + ManifestWriter::MakeWriter(kFormatVersion, kSnapshotId, path, + file_io_, spec, schema_, content)); + for (int i = 0; i < num_files; ++i) { + auto f = std::make_shared(); + f->content = (content == ManifestContent::kDeletes) + ? DataFile::Content::kPositionDeletes + : DataFile::Content::kData; + f->file_path = std::format("data/file-{}-{}.parquet", manifest_counter_, i); + f->file_format = FileFormatType::kParquet; + // Identity spec uses LONG partition values; Bucket spec uses INT + Literal part_val = (spec_id == kSpecId0) ? Literal::Long(i) : Literal::Int(i % 8); + f->partition = PartitionValues(std::vector{part_val}); + f->file_size_in_bytes = 1024; + f->record_count = 10; + f->partition_spec_id = spec_id; + ICEBERG_RETURN_UNEXPECTED(writer->WriteAddedEntry(f)); + } + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + ICEBERG_ASSIGN_OR_RAISE(auto manifest_file, writer->ToManifestFile()); + // Override length so we can control bin-packing behaviour in tests + manifest_file.manifest_length = file_size_override; + return manifest_file; + } + + ManifestWriterFactory MakeWriterFactory() { + return [this](int32_t spec_id, + ManifestContent content) -> Result> { + ++factory_call_count_; + auto spec = spec_id == kSpecId0 ? spec0_ : spec1_; + auto path = std::format("merged-{}.avro", manifest_counter_++); + return ManifestWriter::MakeWriter(kFormatVersion, kSnapshotId, path, file_io_, spec, + schema_, content); + }; + } + + // Count total entries across all manifests. + Result CountEntries(const std::vector& manifests) { + int total = 0; + for (const auto& m : manifests) { + auto spec = m.partition_spec_id == kSpecId0 ? spec0_ : spec1_; + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(m, file_io_, schema_, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + total += static_cast(entries.size()); + } + return total; + } + + std::shared_ptr file_io_; + std::shared_ptr schema_; + std::shared_ptr spec0_; + std::shared_ptr spec1_; + std::shared_ptr metadata_; + int manifest_counter_ = 0; + int factory_call_count_ = 0; +}; + +TEST_F(ManifestMergeManagerTest, MergeDisabled) { + ICEBERG_UNWRAP_OR_FAIL(auto m0, WriteManifest(kSpecId0, 1)); + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(kSpecId0, 1)); + ICEBERG_UNWRAP_OR_FAIL(auto m2, WriteManifest(kSpecId0, 1)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/2, /*enabled=*/false); + ICEBERG_UNWRAP_OR_FAIL( + auto result, mgr.MergeManifests({m0, m1}, {m2}, kSnapshotId, *metadata_, file_io_, + MakeWriterFactory())); + // merge disabled → all 3 manifests returned, factory never called + EXPECT_EQ(result.size(), 3U); + EXPECT_EQ(factory_call_count_, 0); +} + +TEST_F(ManifestMergeManagerTest, BelowMinCountThreshold) { + ICEBERG_UNWRAP_OR_FAIL(auto m0, WriteManifest(kSpecId0, 1)); + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(kSpecId0, 1)); + + // min_count=3, only 2 manifests total → no merge + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/3, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.MergeManifests({m0}, {m1}, kSnapshotId, *metadata_, file_io_, + MakeWriterFactory())); + EXPECT_EQ(result.size(), 2U); + EXPECT_EQ(factory_call_count_, 0); +} + +TEST_F(ManifestMergeManagerTest, MergeOccursAtThreshold) { + // 3 small manifests (each 100 bytes), target=1024 → all fit in one bin + ICEBERG_UNWRAP_OR_FAIL(auto m0, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m2, WriteManifest(kSpecId0, 1, /*size=*/100)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/3, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL( + auto result, mgr.MergeManifests({m0, m1}, {m2}, kSnapshotId, *metadata_, file_io_, + MakeWriterFactory())); + // All 3 merged into 1 manifest (total 3 entries) + EXPECT_EQ(result.size(), 1U); + ICEBERG_UNWRAP_OR_FAIL(auto count1, CountEntries(result)); + EXPECT_EQ(count1, 3); +} + +TEST_F(ManifestMergeManagerTest, OversizedManifestPassedThrough) { + // m_large exceeds target → must not be merged; m_small fits + ICEBERG_UNWRAP_OR_FAIL(auto m_large, WriteManifest(kSpecId0, 2, /*size=*/2000)); + ICEBERG_UNWRAP_OR_FAIL(auto m_small, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m_small2, WriteManifest(kSpecId0, 1, /*size=*/100)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/2, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.MergeManifests({m_large, m_small}, {m_small2}, kSnapshotId, + *metadata_, file_io_, MakeWriterFactory())); + // m_large is oversized and acts as a bin boundary — the two small manifests on either + // side of it are never merged together. m_small2 (the newest) is also protected by + // minCountToMerge (size 1 < 2). All three remain separate. + EXPECT_EQ(result.size(), 3U); + ICEBERG_UNWRAP_OR_FAIL(auto count2, CountEntries(result)); + EXPECT_EQ(count2, 4); // 2 + 1 + 1 +} + +TEST_F(ManifestMergeManagerTest, CrossSpecManifestsNotMerged) { + // Manifests with different spec IDs must never be merged together + ICEBERG_UNWRAP_OR_FAIL(auto m_spec0a, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m_spec0b, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m_spec1a, WriteManifest(kSpecId1, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m_spec1b, WriteManifest(kSpecId1, 1, /*size=*/100)); + + // With 4 manifests (target large enough for each pair), we get 2 merged outputs + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/2, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL( + auto result, + mgr.MergeManifests({m_spec0a, m_spec1a}, {m_spec0b, m_spec1b}, kSnapshotId, + *metadata_, file_io_, MakeWriterFactory())); + EXPECT_EQ(result.size(), 2U); + // Verify spec IDs are preserved per output manifest + for (const auto& m : result) { + EXPECT_THAT(m.partition_spec_id, ::testing::AnyOf(kSpecId0, kSpecId1)); + } +} + +TEST_F(ManifestMergeManagerTest, WriterFactoryCalledOncePerMergedManifest) { + // 4 small manifests in two groups → 2 merged outputs → factory called twice + ICEBERG_UNWRAP_OR_FAIL(auto m0, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m2, WriteManifest(kSpecId1, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m3, WriteManifest(kSpecId1, 1, /*size=*/100)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/2, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.MergeManifests({m0, m2}, {m1, m3}, kSnapshotId, *metadata_, + file_io_, MakeWriterFactory())); + EXPECT_EQ(result.size(), 2U); + EXPECT_EQ(factory_call_count_, 2); +} + +TEST_F(ManifestMergeManagerTest, MixedContentManifestsNotMerged) { + // Data and delete manifests sharing the same spec_id must never be merged together. + // The grouping key is (spec_id, content), so they land in separate bins. + ICEBERG_UNWRAP_OR_FAIL( + auto d0, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kData)); + ICEBERG_UNWRAP_OR_FAIL( + auto d1, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kData)); + ICEBERG_UNWRAP_OR_FAIL( + auto del0, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + ICEBERG_UNWRAP_OR_FAIL( + auto del1, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/2, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL( + auto result, mgr.MergeManifests({d0, del0}, {d1, del1}, kSnapshotId, *metadata_, + file_io_, MakeWriterFactory())); + // 2 data → 1 merged data manifest; 2 delete → 1 merged delete manifest + EXPECT_EQ(result.size(), 2U); + int data_count = 0; + int delete_count = 0; + for (const auto& m : result) { + if (m.content == ManifestContent::kData) ++data_count; + if (m.content == ManifestContent::kDeletes) ++delete_count; + } + EXPECT_EQ(data_count, 1); + EXPECT_EQ(delete_count, 1); +} + +TEST_F(ManifestMergeManagerTest, MixedContentUsesFirstManifestPerContent) { + ICEBERG_UNWRAP_OR_FAIL( + auto d0, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kData)); + ICEBERG_UNWRAP_OR_FAIL( + auto d1, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kData)); + ICEBERG_UNWRAP_OR_FAIL( + auto del0, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + ICEBERG_UNWRAP_OR_FAIL( + auto del1, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + + // Each content type's newest manifest must be protected by the threshold + // independently. + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/3, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL( + auto result, mgr.MergeManifests({d0, del0}, {d1, del1}, kSnapshotId, *metadata_, + file_io_, MakeWriterFactory())); + + // Each content type has exactly two manifests, below min_count=3, so neither pair + // should be merged. + ASSERT_EQ(result.size(), 4U); + int data_count = 0; + int delete_count = 0; + for (const auto& manifest : result) { + if (manifest.content == ManifestContent::kData) { + ++data_count; + } else if (manifest.content == ManifestContent::kDeletes) { + ++delete_count; + } + } + EXPECT_EQ(data_count, 2); + EXPECT_EQ(delete_count, 2); +} + +TEST_F(ManifestMergeManagerTest, DeleteManifestsMerged) { + // Delete manifests are bin-packed and merged just like data manifests. + ICEBERG_UNWRAP_OR_FAIL( + auto del0, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + ICEBERG_UNWRAP_OR_FAIL( + auto del1, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + ICEBERG_UNWRAP_OR_FAIL( + auto del2, WriteManifest(kSpecId0, 1, /*size=*/100, ManifestContent::kDeletes)); + + ManifestMergeManager mgr(/*target=*/1024, /*min_count=*/3, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL(auto result, + mgr.MergeManifests({del0, del1}, {del2}, kSnapshotId, *metadata_, + file_io_, MakeWriterFactory())); + EXPECT_EQ(result.size(), 1U); + EXPECT_EQ(result[0].content, ManifestContent::kDeletes); + ICEBERG_UNWRAP_OR_FAIL(auto count, CountEntries(result)); + EXPECT_EQ(count, 3); +} + +TEST_F(ManifestMergeManagerTest, PackEndOlderManifestsMergedNotNewest) { + // packEnd semantics: for [m0_new, m1_old, m2_old] with target=250 (pairs fit but + // triples don't), packing from the end merges m1+m2 (the older pair) and leaves + // m0 (the newest) in its own under-filled bin at the front of the output. + // This is the opposite of naive forward packing, which would merge m0+m1. + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m2, WriteManifest(kSpecId0, 1, /*size=*/100)); + ICEBERG_UNWRAP_OR_FAIL(auto m0, WriteManifest(kSpecId0, 1, /*size=*/100)); + + // target=250 fits two 100-byte manifests but not three. + // min_count=3 so m0's single-element bin is kept as-is (below threshold). + ManifestMergeManager mgr(/*target=*/250, /*min_count=*/3, /*enabled=*/true); + ICEBERG_UNWRAP_OR_FAIL( + auto result, mgr.MergeManifests({m1, m2}, {m0}, kSnapshotId, *metadata_, file_io_, + MakeWriterFactory())); + // Expected: [m0 (pass-through), merged(m1+m2)] + ASSERT_EQ(result.size(), 2U); + // First output is the newest manifest m0, passed through unchanged (under-filled bin). + EXPECT_EQ(result[0].manifest_length, m0.manifest_length); + // Second output is the merged older pair — it must be a newly written manifest + // (different path than either original). + EXPECT_NE(result[1].manifest_path, m1.manifest_path); + EXPECT_NE(result[1].manifest_path, m2.manifest_path); + ICEBERG_UNWRAP_OR_FAIL(auto count, CountEntries(result)); + EXPECT_EQ(count, 3); +} + +} // namespace iceberg diff --git a/src/iceberg/test/merging_snapshot_update_test.cc b/src/iceberg/test/merging_snapshot_update_test.cc new file mode 100644 index 000000000..5ce4a8ad4 --- /dev/null +++ b/src/iceberg/test/merging_snapshot_update_test.cc @@ -0,0 +1,735 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/update/merging_snapshot_update.h" + +#include +#include +#include +#include + +#include +#include + +#include "iceberg/avro/avro_register.h" +#include "iceberg/constants.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/partition_spec.h" +#include "iceberg/row/partition_values.h" +#include "iceberg/schema.h" +#include "iceberg/snapshot.h" +#include "iceberg/table.h" +#include "iceberg/table_metadata.h" +#include "iceberg/table_properties.h" +#include "iceberg/test/matchers.h" +#include "iceberg/test/update_test_base.h" +#include "iceberg/transaction.h" +#include "iceberg/update/fast_append.h" +#include "iceberg/update/update_properties.h" +#include "iceberg/util/macros.h" + +namespace iceberg { + +/// \brief Concrete subclass of MergingSnapshotUpdate for testing. +class TestMergeAppend : public MergingSnapshotUpdate { + public: + static Result> Make(std::string table_name, + std::shared_ptr table) { + ICEBERG_ASSIGN_OR_RAISE( + auto ctx, TransactionContext::Make(std::move(table), TransactionKind::kUpdate)); + return std::unique_ptr( + new TestMergeAppend(std::move(table_name), std::move(ctx))); + } + + std::string operation() override { return "append"; } + + // Expose protected API for test access + Status AddFile(std::shared_ptr file) { return AddDataFile(std::move(file)); } + Status AddDelete(std::shared_ptr file) { + return AddDeleteFile(std::move(file)); + } + Status RemoveDataFile(std::shared_ptr file) { + return DeleteDataFile(std::move(file)); + } + Status RemoveDeleteFile(std::shared_ptr file) { + return DeleteDeleteFile(std::move(file)); + } + Status AppendManifest(ManifestFile manifest) { + return AddManifest(std::move(manifest)); + } + Result> DataSpec() const { + return MergingSnapshotUpdate::DataSpec(); + } + void SetDataSeqNumber(int64_t seq) { SetNewDataFilesDataSequenceNumber(seq); } + + bool HasDataFiles() const { return AddsDataFiles(); } + bool HasDeleteFiles() const { return AddsDeleteFiles(); } + bool HasDataDeletes() const { return DeletesDataFiles(); } + + private: + TestMergeAppend(std::string table_name, std::shared_ptr ctx) + : MergingSnapshotUpdate(std::move(table_name), std::move(ctx)) {} +}; + +class MergingSnapshotUpdateTest : public MinimalUpdateTestBase { + protected: + static void SetUpTestSuite() { avro::RegisterAll(); } + + void SetUp() override { + MinimalUpdateTestBase::SetUp(); + + ICEBERG_UNWRAP_OR_FAIL(spec_, table_->spec()); + ICEBERG_UNWRAP_OR_FAIL(schema_, table_->schema()); + + file_a_ = MakeDataFile("/data/file_a.parquet", /*partition_x=*/1L); + file_b_ = MakeDataFile("/data/file_b.parquet", /*partition_x=*/2L); + } + + std::shared_ptr MakeDataFile(const std::string& path, int64_t partition_x) { + auto f = std::make_shared(); + f->content = DataFile::Content::kData; + f->file_path = table_location_ + path; + f->file_format = FileFormatType::kParquet; + f->partition = PartitionValues(std::vector{Literal::Long(partition_x)}); + f->file_size_in_bytes = 1024; + f->record_count = 100; + f->partition_spec_id = spec_->spec_id(); + return f; + } + + std::shared_ptr MakeDeleteFile(const std::string& path, int64_t partition_x) { + auto f = MakeDataFile(path, partition_x); + f->content = DataFile::Content::kPositionDeletes; + return f; + } + + Result> NewMergeAppend() { + return TestMergeAppend::Make(TableName(), table_); + } + + // Commit file_a_ with FastAppend and refresh the table. + void CommitFileA() { + ICEBERG_UNWRAP_OR_FAIL(auto fa, table_->NewFastAppend()); + fa->AppendFile(file_a_); + EXPECT_THAT(fa->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + } + + // Read all entries from a list of ManifestFiles. + Result> ReadAllEntries( + const std::vector& manifests, const TableMetadata& metadata) { + std::vector result; + for (const auto& m : manifests) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, metadata.PartitionSpecById(m.partition_spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(m, file_io_, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + result.insert(result.end(), entries.begin(), entries.end()); + } + return result; + } + + // Write a manifest file containing the given data files. + // Returns a ManifestFile with added_snapshot_id = kInvalidSnapshotId so it + // is eligible for snapshot ID inheritance. + Result WriteManifest( + const std::string& path, const std::vector>& files) { + ICEBERG_ASSIGN_OR_RAISE( + auto writer, + ManifestWriter::MakeWriter(/*format_version=*/2, kInvalidSnapshotId, path, + file_io_, spec_, schema_, ManifestContent::kData)); + for (const auto& f : files) { + ManifestEntry entry; + entry.status = ManifestStatus::kAdded; + entry.snapshot_id = std::nullopt; + entry.data_file = f; + ICEBERG_RETURN_UNEXPECTED(writer->WriteAddedEntry(entry)); + } + ICEBERG_RETURN_UNEXPECTED(writer->Close()); + return writer->ToManifestFile(); + } + + std::shared_ptr spec_; + std::shared_ptr schema_; + std::shared_ptr file_a_; + std::shared_ptr file_b_; +}; + +// ------------------------------------------------------------------------- +// State query tests +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, AddsDataFiles_InitiallyFalse) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_FALSE(op->HasDataFiles()); + EXPECT_FALSE(op->HasDeleteFiles()); + EXPECT_FALSE(op->HasDataDeletes()); +} + +TEST_F(MergingSnapshotUpdateTest, AddsDataFiles_TrueAfterAdd) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_TRUE(op->HasDataFiles()); + EXPECT_FALSE(op->HasDeleteFiles()); +} + +TEST_F(MergingSnapshotUpdateTest, AddsDeleteFiles_TrueAfterAdd) { + auto del_file = MakeDeleteFile("/delete/del_a.parquet", 1L); + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(del_file), IsOk()); + EXPECT_FALSE(op->HasDataFiles()); + EXPECT_TRUE(op->HasDeleteFiles()); +} + +TEST_F(MergingSnapshotUpdateTest, DeletesDataFiles_TrueAfterRegisterDelete) { + CommitFileA(); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->RemoveDataFile(file_a_), IsOk()); + EXPECT_TRUE(op->HasDataDeletes()); +} + +// ------------------------------------------------------------------------- +// Apply / Commit tests +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, CommitNewDataFile) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "1"); + EXPECT_EQ(snapshot->summary.at("added-records"), "100"); +} + +TEST_F(MergingSnapshotUpdateTest, CommitMultipleDataFiles) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "2"); + EXPECT_EQ(snapshot->summary.at("added-records"), "200"); +} + +TEST_F(MergingSnapshotUpdateTest, CommitDataFileAndDeleteFile) { + auto del_file = MakeDeleteFile("/delete/del_a.parquet", 1L); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->AddDelete(del_file), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + // Data file summary + EXPECT_EQ(snapshot->summary.at("added-data-files"), "1"); +} + +TEST_F(MergingSnapshotUpdateTest, CommitPreservesExistingManifests) { + // First append: file_a + CommitFileA(); + + // Second merge append: file_b + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + // Both data files should be visible — 1 existing + 1 new + EXPECT_EQ(snapshot->summary.at("total-data-files"), "2"); +} + +TEST_F(MergingSnapshotUpdateTest, CommitDeletesDataFile) { + CommitFileA(); + + // Remove file_a via merging snapshot update + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->RemoveDataFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at("total-data-files"), "0"); + EXPECT_EQ(snapshot->summary.at("deleted-data-files"), "1"); +} + +TEST_F(MergingSnapshotUpdateTest, SetNewDataFilesDataSequenceNumber) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + op->SetDataSeqNumber(42); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "1"); +} + +// ------------------------------------------------------------------------- +// CleanUncommitted test +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, CleanUncommitted_ClearsCaches) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + // CleanUncommitted with an empty set should delete everything and not crash. + op->CleanUncommitted({}); +} + +// ------------------------------------------------------------------------- +// Delete file summary tests +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, CommitDeleteFile_SummaryHasAddedDeleteFiles) { + auto del_file = MakeDeleteFile("/delete/del_a.parquet", 1L); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(del_file), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kAddedDeleteFiles), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kAddedPosDeleteFiles), "1"); + EXPECT_EQ(snapshot->summary.count(SnapshotSummaryFields::kRemovedDeleteFiles), 0); +} + +// Covers the bug where deleted delete files were not tracked in the snapshot summary. +// Java: deleteFilterManager.buildSummary(filteredDeletes) handles this. +// C++: loop over delete_filter_manager_.FilesToBeDeleted() after Step 3. +TEST_F(MergingSnapshotUpdateTest, CommitDeletesDeleteFile_SummaryHasRemovedDeleteFiles) { + // Step 1: commit a delete file. + auto del_file = MakeDeleteFile("/delete/del_a.parquet", 1L); + { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(del_file), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + } + + // Step 2: commit a new snapshot that removes the delete file. + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->RemoveDeleteFile(del_file), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kRemovedDeleteFiles), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kRemovedPosDeleteFiles), "1"); + EXPECT_EQ(snapshot->summary.count(SnapshotSummaryFields::kAddedDeleteFiles), 0); +} + +// ------------------------------------------------------------------------- +// Deduplication test +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, DuplicateDataFile_OnlyCountedOnce) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); // duplicate — should be ignored + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kAddedDataFiles), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kTotalDataFiles), "1"); +} + +// ------------------------------------------------------------------------- +// ValidateNewDeleteFile format version tests +// ------------------------------------------------------------------------- + +/// \brief V1-table test fixture — deletes are not supported in format v1. +class MergingSnapshotUpdateV1Test : public UpdateTestBase { + protected: + std::string MetadataResource() const override { return "TableMetadataV1Valid.json"; } + std::string TableName() const override { return "v1_test_table"; } + + void SetUp() override { + UpdateTestBase::SetUp(); + ICEBERG_UNWRAP_OR_FAIL(spec_, table_->spec()); + } + + std::shared_ptr MakeDeleteFile(const std::string& path) { + auto f = std::make_shared(); + f->content = DataFile::Content::kPositionDeletes; + f->file_path = table_location_ + path; + f->file_format = FileFormatType::kParquet; + f->partition = PartitionValues(std::vector{Literal::Long(1L)}); + f->file_size_in_bytes = 512; + f->record_count = 10; + f->partition_spec_id = spec_->spec_id(); + return f; + } + + Result> NewMergeAppend() { + return TestMergeAppend::Make(TableName(), table_); + } + + std::shared_ptr spec_; +}; + +TEST_F(MergingSnapshotUpdateV1Test, ValidateNewDeleteFile_V1Rejected) { + auto del_file = MakeDeleteFile("/delete/del_a.parquet"); + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(del_file), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, ValidateNewDeleteFile_V2RejectsDeletionVector) { + // Position delete with referenced_data_file set = deletion vector, not allowed in v2. + auto del_file = MakeDeleteFile("/delete/del_a.parquet", 1L); + del_file->referenced_data_file = table_location_ + "/data/file_a.parquet"; + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(del_file), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, ValidateNewDeleteFile_V2AllowsEqualityDelete) { + auto eq_del = MakeDeleteFile("/delete/eq_del.parquet", 1L); + eq_del->content = DataFile::Content::kEqualityDeletes; + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddDelete(eq_del), IsOk()); +} + +// ------------------------------------------------------------------------- +// AddManifest — invalid manifest rejection +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, AddManifest_RejectsDeleteManifest) { + // Build a ManifestFile with content = kDeletes + ManifestFile del_manifest; + del_manifest.manifest_path = table_location_ + "/metadata/del.avro"; + del_manifest.content = ManifestContent::kDeletes; + del_manifest.added_snapshot_id = kInvalidSnapshotId; + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(del_manifest), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, AddManifest_RejectsManifestWithExistingFiles) { + // Construct a ManifestFile that reports existing files without writing to disk. + ManifestFile manifest; + manifest.manifest_path = table_location_ + "/metadata/existing.avro"; + manifest.content = ManifestContent::kData; + manifest.added_snapshot_id = kInvalidSnapshotId; + manifest.existing_files_count = 1; // has_existing_files() returns true + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(manifest), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, AddManifest_RejectsManifestWithDeletedFiles) { + ManifestFile manifest; + manifest.manifest_path = table_location_ + "/metadata/deleted.avro"; + manifest.content = ManifestContent::kData; + manifest.added_snapshot_id = kInvalidSnapshotId; + manifest.deleted_files_count = 1; // has_deleted_files() returns true + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(manifest), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, AddManifest_RejectsManifestWithAssignedSnapshotId) { + ManifestFile manifest; + manifest.manifest_path = table_location_ + "/metadata/snap.avro"; + manifest.content = ManifestContent::kData; + manifest.added_snapshot_id = 12345; // already assigned + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(manifest), IsError(ErrorKind::kInvalidArgument)); +} + +TEST_F(MergingSnapshotUpdateTest, AddManifest_RejectsManifestWithFirstRowId) { + ManifestFile manifest; + manifest.manifest_path = table_location_ + "/metadata/rowid.avro"; + manifest.content = ManifestContent::kData; + manifest.added_snapshot_id = kInvalidSnapshotId; + manifest.first_row_id = 0; // assigned first_row_id + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(manifest), IsError(ErrorKind::kInvalidArgument)); +} + +// ------------------------------------------------------------------------- +// AddManifest — basic commit (inherit path: v2 with can_inherit_snapshot_id) +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, AppendManifest_EmptyTable) { + auto path = table_location_ + "/metadata/input.avro"; + ICEBERG_UNWRAP_OR_FAIL(auto manifest, WriteManifest(path, {file_a_, file_b_})); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(manifest), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + + // In v2 with snapshot ID inheritance, the manifest path is reused directly. + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + ASSERT_EQ(data_manifests.size(), 1); + + EXPECT_EQ(snapshot->summary.at("added-data-files"), "2"); + EXPECT_EQ(snapshot->summary.at("total-data-files"), "2"); +} + +TEST_F(MergingSnapshotUpdateTest, AppendManifest_WithDataFiles) { + // Mix AddDataFile + AddManifest — should produce 2 manifests. + auto path = table_location_ + "/metadata/input.avro"; + ICEBERG_UNWRAP_OR_FAIL(auto manifest, WriteManifest(path, {file_a_, file_b_})); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); // file_b_ staged directly + EXPECT_THAT(op->AppendManifest(manifest), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + // Written manifest (file_b_) + appended manifest (file_a_, file_b_) + EXPECT_EQ(data_manifests.size(), 2); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "3"); +} + +// ------------------------------------------------------------------------- +// AddManifest — merge behavior +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, AppendManifest_MergeWithMinCountOne) { + // Set min-count-to-merge = 1 so all manifests are merged. + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestMinMergeCount.key()), "1"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + auto path = table_location_ + "/metadata/input.avro"; + ICEBERG_UNWRAP_OR_FAIL(auto manifest, WriteManifest(path, {file_a_, file_b_})); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->AppendManifest(manifest), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + // Both manifests merged into one. + EXPECT_EQ(data_manifests.size(), 1); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "3"); +} + +TEST_F(MergingSnapshotUpdateTest, AppendManifest_DoNotMergeMinCount) { + // Set min-count-to-merge = 4 so 3 manifests are not merged. + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestMinMergeCount.key()), "4"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + auto path1 = table_location_ + "/metadata/m1.avro"; + auto path2 = table_location_ + "/metadata/m2.avro"; + auto path3 = table_location_ + "/metadata/m3.avro"; + ICEBERG_UNWRAP_OR_FAIL(auto m1, WriteManifest(path1, {file_a_})); + ICEBERG_UNWRAP_OR_FAIL(auto m2, WriteManifest(path2, {file_b_})); + ICEBERG_UNWRAP_OR_FAIL( + auto m3, WriteManifest(path3, {MakeDataFile("/data/file_c.parquet", 3L)})); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AppendManifest(m1), IsOk()); + EXPECT_THAT(op->AppendManifest(m2), IsOk()); + EXPECT_THAT(op->AppendManifest(m3), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + // Below min-count-to-merge threshold — all 3 pass through unchanged. + EXPECT_EQ(data_manifests.size(), 3); + EXPECT_EQ(snapshot->summary.at("added-data-files"), "3"); +} + +// ------------------------------------------------------------------------- +// Manifest merge — data files only +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, ManifestMerge_MergesIntoOne) { + // Set min-count-to-merge = 1 so every append triggers a merge. + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestMinMergeCount.key()), "1"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + // Snapshot 1: file_a_ + CommitFileA(); + + // Snapshot 2: file_b_ — should merge with existing manifest. + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + EXPECT_EQ(data_manifests.size(), 1); + EXPECT_EQ(snapshot->summary.at("total-data-files"), "2"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsReplaced), "1"); +} + +TEST_F(MergingSnapshotUpdateTest, ManifestMerge_DoesNotMergeWhenBelowMinCount) { + // Default min-count-to-merge = 100, so manifests are not merged. + CommitFileA(); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + EXPECT_EQ(data_manifests.size(), 2); + EXPECT_EQ(snapshot->summary.at("total-data-files"), "2"); +} + +TEST_F(MergingSnapshotUpdateTest, ManifestMerge_DoesNotMergeWhenSizeTargetTooSmall) { + // Set a tiny size target so manifests never merge. + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestTargetSizeBytes.key()), "10"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + CommitFileA(); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + ICEBERG_UNWRAP_OR_FAIL(auto data_manifests, + SnapshotCache(snapshot.get()).DataManifests(file_io_)); + EXPECT_EQ(data_manifests.size(), 2); +} + +// ------------------------------------------------------------------------- +// Manifest count summary +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, Summary_ManifestCountsOnFirstCommit) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsCreated), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsReplaced), "0"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsKept), "0"); +} + +TEST_F(MergingSnapshotUpdateTest, Summary_ManifestCountsOnSecondCommitNoMerge) { + CommitFileA(); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + // 1 new manifest created, 1 existing manifest kept, 0 replaced. + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsCreated), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsReplaced), "0"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsKept), "1"); +} + +TEST_F(MergingSnapshotUpdateTest, Summary_ManifestCountsAfterMerge) { + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestMinMergeCount.key()), "1"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + CommitFileA(); + + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + // 1 merged output created, 1 existing manifest replaced, 0 kept. + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsCreated), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsReplaced), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsKept), "0"); +} + +TEST_F(MergingSnapshotUpdateTest, Summary_ManifestCountsAfterDelete) { + ICEBERG_UNWRAP_OR_FAIL(auto props, table_->NewUpdateProperties()); + props->Set(std::string(TableProperties::kManifestMinMergeCount.key()), "1"); + EXPECT_THAT(props->Commit(), IsOk()); + EXPECT_THAT(table_->Refresh(), IsOk()); + + CommitFileA(); + + // Delete file_a_ — filter manager rewrites the manifest. + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + EXPECT_THAT(op->RemoveDataFile(file_a_), IsOk()); + EXPECT_THAT(op->Commit(), IsOk()); + + EXPECT_THAT(table_->Refresh(), IsOk()); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, table_->current_snapshot()); + // Filter rewrites 1 manifest (replaced), merge produces 1 output (created). + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsReplaced), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsCreated), "1"); + EXPECT_EQ(snapshot->summary.at(SnapshotSummaryFields::kManifestsKept), "0"); +} + +// ------------------------------------------------------------------------- +// DataSpec — multiple partition specs +// ------------------------------------------------------------------------- + +TEST_F(MergingSnapshotUpdateTest, DataSpec_ThrowsWithMultipleSpecs) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + // file_a_ and file_b_ both use spec_id 0 — DataSpec() should succeed. + EXPECT_THAT(op->AddFile(file_a_), IsOk()); + EXPECT_THAT(op->AddFile(file_b_), IsOk()); + EXPECT_THAT(op->DataSpec(), IsOk()); +} + +TEST_F(MergingSnapshotUpdateTest, DataSpec_ThrowsWhenEmpty) { + ICEBERG_UNWRAP_OR_FAIL(auto op, NewMergeAppend()); + // No files added — DataSpec() should fail. + EXPECT_THAT(op->DataSpec(), IsError(ErrorKind::kInvalidArgument)); +} + +} // namespace iceberg diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index e168d08bf..1acb46e9b 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -88,6 +88,7 @@ iceberg_tests = { 'data_file_set_test.cc', 'decimal_test.cc', 'endian_test.cc', + 'file_io_test.cc', 'formatter_test.cc', 'lazy_test.cc', 'location_util_test.cc', diff --git a/src/iceberg/test/parquet_test.cc b/src/iceberg/test/parquet_test.cc index 65a4602d8..70fb9880f 100644 --- a/src/iceberg/test/parquet_test.cc +++ b/src/iceberg/test/parquet_test.cc @@ -18,6 +18,9 @@ */ #include +#include +#include +#include #include #include @@ -26,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -124,6 +128,27 @@ void DoRoundtrip(std::shared_ptr<::arrow::Array> data, std::shared_ptr s ASSERT_TRUE(out != nullptr) << "Reader.Next() returned no data"; } +struct ParquetCodec { + std::string name; + ::arrow::Compression::type compression; +}; + +std::optional FirstUnavailableParquetCodec() { + const std::vector codecs = { + {.name = "snappy", .compression = ::arrow::Compression::SNAPPY}, + {.name = "gzip", .compression = ::arrow::Compression::GZIP}, + {.name = "brotli", .compression = ::arrow::Compression::BROTLI}, + {.name = "lz4", .compression = ::arrow::Compression::LZ4}, + {.name = "zstd", .compression = ::arrow::Compression::ZSTD}, + }; + for (const auto& codec : codecs) { + if (!::arrow::util::Codec::IsAvailable(codec.compression)) { + return codec; + } + } + return std::nullopt; +} + } // namespace class ParquetReaderTest : public TempFileTestBase { @@ -461,6 +486,29 @@ TEST_F(ParquetReadWrite, EmptyStruct) { IsError(ErrorKind::kNotImplemented)); } +TEST_F(ParquetReadWrite, RejectsUnavailableCompressionCodec) { + auto unavailable_codec = FirstUnavailableParquetCodec(); + if (!unavailable_codec.has_value()) { + GTEST_SKIP() << "All optional Parquet compression codecs are available"; + } + + auto schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "id", int32())}); + WriterProperties writer_properties; + writer_properties.Set(WriterProperties::kParquetCompression, unavailable_codec->name); + + auto writer = WriterFactoryRegistry::Open( + FileFormatType::kParquet, {.path = "unavailable_codec.parquet", + .schema = schema, + .io = arrow::ArrowFileSystemFileIO::MakeMockFileIO(), + .properties = std::move(writer_properties)}); + + EXPECT_THAT(writer, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(writer, + HasErrorMessage("Parquet compression codec " + unavailable_codec->name + + " is not available in the current build")); +} + TEST_F(ParquetReadWrite, SimpleStructRoundTrip) { auto schema = std::make_shared(std::vector{ SchemaField::MakeOptional(1, "a", diff --git a/src/iceberg/test/table_scan_test.cc b/src/iceberg/test/table_scan_test.cc index e4a3d21f4..11905a870 100644 --- a/src/iceberg/test/table_scan_test.cc +++ b/src/iceberg/test/table_scan_test.cc @@ -17,6 +17,7 @@ * under the License. */ +#include #include #include #include @@ -205,6 +206,30 @@ TEST_P(TableScanTest, TableScanBuilderOptions) { EXPECT_EQ(snapshot->snapshot_id, 1000L); } +TEST_P(TableScanTest, UseRefPreservesInt64SnapshotIds) { + constexpr int64_t kLargeSnapshotId = + static_cast(std::numeric_limits::max()) + 42; + table_metadata_->snapshots.push_back(std::make_shared( + Snapshot{.snapshot_id = kLargeSnapshotId, + .parent_snapshot_id = table_metadata_->current_snapshot_id, + .sequence_number = 2L, + .timestamp_ms = TimePointMsFromUnixMs(1609459201000L), + .manifest_list = "/tmp/metadata/snap-large-2-manifest-list.avro", + .schema_id = schema_->schema_id()})); + table_metadata_->refs["branch-with-large-snapshot-id"] = std::make_shared( + SnapshotRef{.snapshot_id = kLargeSnapshotId, .retention = SnapshotRef::Branch{}}); + + ICEBERG_UNWRAP_OR_FAIL(auto builder, + DataTableScanBuilder::Make(table_metadata_, file_io_)); + builder->UseRef("branch-with-large-snapshot-id"); + ICEBERG_UNWRAP_OR_FAIL(auto scan, builder->Build()); + + ASSERT_TRUE(scan->context().snapshot_id.has_value()); + EXPECT_EQ(scan->context().snapshot_id.value(), kLargeSnapshotId); + ICEBERG_UNWRAP_OR_FAIL(auto snapshot, scan->snapshot()); + EXPECT_EQ(snapshot->snapshot_id, kLargeSnapshotId); +} + TEST_P(TableScanTest, TableScanBuilderValidationErrors) { // Test negative min rows ICEBERG_UNWRAP_OR_FAIL(auto builder, diff --git a/src/iceberg/update/expire_snapshots.cc b/src/iceberg/update/expire_snapshots.cc index ce65882c9..c9ac9e4cd 100644 --- a/src/iceberg/update/expire_snapshots.cc +++ b/src/iceberg/update/expire_snapshots.cc @@ -23,14 +23,16 @@ #include #include #include -#include #include +#include #include +#include #include #include "iceberg/file_io.h" #include "iceberg/manifest/manifest_entry.h" #include "iceberg/manifest/manifest_reader.h" +#include "iceberg/result.h" #include "iceberg/schema.h" #include "iceberg/snapshot.h" #include "iceberg/statistics_file.h" @@ -40,18 +42,23 @@ #include "iceberg/util/error_collector.h" #include "iceberg/util/macros.h" #include "iceberg/util/snapshot_util_internal.h" +#include "iceberg/util/string_util.h" namespace iceberg { namespace { -Result> MakeManifestReader( +Result> MakeManifestReader( const ManifestFile& manifest, const std::shared_ptr& file_io, const TableMetadata& metadata) { + // TODO(gangwu): Build manifest file schemas from PartitionSpec::RawPartitionType + // with UnknownType for dropped source fields instead of requiring the table schema + // to bind every partition source field. Until then, cleanup fails closed when + // historical specs cannot bind to the metadata schema. ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); - ICEBERG_ASSIGN_OR_RAISE(auto spec, - metadata.PartitionSpecById(manifest.partition_spec_id)); - return ManifestReader::Make(manifest, file_io, std::move(schema), std::move(spec)); + TableMetadataCache metadata_cache(&metadata); + ICEBERG_ASSIGN_OR_RAISE(auto specs_by_id, metadata_cache.GetPartitionSpecsById()); + return ManifestReader::Make(manifest, file_io, std::move(schema), specs_by_id.get()); } /// \brief Abstract strategy for cleaning up files after snapshot expiration. @@ -67,31 +74,43 @@ class FileCleanupStrategy { /// /// \param metadata_before_expiration Table metadata before expiration. /// \param metadata_after_expiration Table metadata after expiration. - /// \param expired_snapshot_ids Snapshot IDs that were expired during this operation. /// \param level Controls which types of files are eligible for deletion. virtual Status CleanFiles(const TableMetadata& metadata_before_expiration, const TableMetadata& metadata_after_expiration, - const std::unordered_set& expired_snapshot_ids, CleanupLevel level) = 0; protected: - /// \brief Delete a single file - void DeleteFile(const std::string& path) { - try { - if (delete_func_) { - delete_func_(path); - } else { - std::ignore = file_io_->DeleteFile(path); + /// \brief Snapshot IDs present in `before` but not in `after`. + static std::unordered_set ExpiredSnapshotIds(const TableMetadata& before, + const TableMetadata& after) { + std::unordered_set after_ids; + after_ids.reserve(after.snapshots.size()); + for (const auto& s : after.snapshots) { + if (s) after_ids.insert(s->snapshot_id); + } + std::unordered_set expired; + expired.reserve(before.snapshots.size()); + for (const auto& s : before.snapshots) { + if (s && !after_ids.contains(s->snapshot_id)) { + expired.insert(s->snapshot_id); } - } catch (...) { - /// TODO(shangxinli): add retry } + return expired; } - /// TODO(shangxinli): Add bulk deletion + /// \brief Delete files at the given locations. void DeleteFiles(const std::unordered_set& paths) { - for (const auto& path : paths) { - DeleteFile(path); + try { + if (delete_func_) { + for (const auto& path : paths) { + delete_func_(path); + } + } else { + std::vector path_list(paths.begin(), paths.end()); + std::ignore = file_io_->DeleteFiles(path_list); + } + } catch (...) { + // TODO(shangxinli): add retry } } @@ -149,8 +168,10 @@ class ReachableFileCleanup : public FileCleanupStrategy { Status CleanFiles(const TableMetadata& metadata_before_expiration, const TableMetadata& metadata_after_expiration, - const std::unordered_set& expired_snapshot_ids, CleanupLevel level) override { + const auto expired_snapshot_ids = + ExpiredSnapshotIds(metadata_before_expiration, metadata_after_expiration); + std::unordered_set retained_snapshot_ids; for (const auto& snapshot : metadata_after_expiration.snapshots) { if (snapshot) { @@ -182,7 +203,7 @@ class ReachableFileCleanup : public FileCleanupStrategy { if (level == CleanupLevel::kAll) { // Deleting data files auto data_files_to_delete = FindDataFilesToDelete( - metadata_after_expiration, manifests_to_delete, current_manifests); + metadata_before_expiration, manifests_to_delete, current_manifests); DeleteFiles(data_files_to_delete); } @@ -195,8 +216,7 @@ class ReachableFileCleanup : public FileCleanupStrategy { DeleteFiles(manifest_lists_to_delete); // Deleting statistics files - if (HasAnyStatisticsFiles(metadata_before_expiration) || - HasAnyStatisticsFiles(metadata_after_expiration)) { + if (HasAnyStatisticsFiles(metadata_before_expiration)) { DeleteFiles( StatisticsFilesToDelete(metadata_before_expiration, metadata_after_expiration)); } @@ -262,7 +282,7 @@ class ReachableFileCleanup : public FileCleanupStrategy { "Cannot read data file paths from a delete manifest: {}", manifest.manifest_path); - /// TODO(shangxinli): optimize by only reading file paths + // TODO(shangxinli): optimize by only reading file paths ICEBERG_ASSIGN_OR_RAISE(auto reader, MakeManifestReader(manifest, file_io_, metadata)); ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->LiveEntries()); @@ -331,6 +351,296 @@ class ReachableFileCleanup : public FileCleanupStrategy { } }; +/// \brief Incremental file cleanup strategy for simple linear-ancestry expirations. +/// +/// Only safe when: +/// * No snapshot IDs were explicitly listed for expiration. +/// * No removed snapshots lived outside the current main ancestry. +/// * No retained snapshots live outside the current main ancestry. +/// +/// Each manifest is attributed to its writer snapshot via added_snapshot_id, so +/// two snapshot passes are enough -- one over retained snapshots to learn which +/// manifests are still live, one over expired snapshots to learn which manifests, +/// manifest lists, and data files to drop. Cherry-pick protection via +/// SnapshotSummaryFields::kSourceSnapshotId prevents removing data that was +/// logically introduced by a snapshot whose changes are still present in the +/// current state under a different id. +/// +/// TODO(shangxinli): Add multi-threaded manifest reading and file deletion support. +class IncrementalFileCleanup : public FileCleanupStrategy { + public: + using FileCleanupStrategy::FileCleanupStrategy; + + Status CleanFiles(const TableMetadata& metadata_before_expiration, + const TableMetadata& metadata_after_expiration, + CleanupLevel level) override { + const auto expired_snapshot_ids = + ExpiredSnapshotIds(metadata_before_expiration, metadata_after_expiration); + if (expired_snapshot_ids.empty()) { + return {}; + } + + std::unordered_set valid_ids; + valid_ids.reserve(metadata_after_expiration.snapshots.size()); + for (const auto& snapshot : metadata_after_expiration.snapshots) { + if (snapshot) { + valid_ids.insert(snapshot->snapshot_id); + } + } + + auto current_result = metadata_before_expiration.SnapshotById( + metadata_before_expiration.current_snapshot_id); + if (!current_result.has_value() || current_result.value() == nullptr) { + return {}; + } + + // Only delete files removed by ancestors of the current table state. + auto ancestors_result = SnapshotUtil::AncestorsOf( + current_result.value()->snapshot_id, [&metadata_before_expiration](int64_t id) { + return metadata_before_expiration.SnapshotById(id); + }); + if (!ancestors_result.has_value()) { + return {}; + } + std::unordered_set ancestor_ids; + ancestor_ids.reserve(ancestors_result.value().size()); + for (const auto& ancestor : ancestors_result.value()) { + if (ancestor) ancestor_ids.insert(ancestor->snapshot_id); + } + + // Protect snapshots whose changes were picked into the current ancestry. + std::unordered_set picked_ancestor_snapshot_ids; + picked_ancestor_snapshot_ids.reserve(ancestor_ids.size()); + for (const auto& ancestor : ancestors_result.value()) { + if (!ancestor) continue; + const auto& summary = ancestor->summary; + auto it = summary.find(SnapshotSummaryFields::kSourceSnapshotId); + if (it == summary.end()) continue; + ICEBERG_ASSIGN_OR_RAISE(auto source_id, + StringUtils::ParseNumber(it->second)); + picked_ancestor_snapshot_ids.insert(source_id); + } + + // Find manifests still referenced by a valid snapshot but written by an + // expired snapshot. Their deleted entries point at data files now safe to + // remove and become candidates for manifests_to_scan below. + std::unordered_set valid_manifests; + std::unordered_set manifests_to_scan; + manifests_to_scan.reserve(expired_snapshot_ids.size()); + for (const auto& snapshot : metadata_after_expiration.snapshots) { + if (!snapshot) continue; + SnapshotCache snapshot_cache(snapshot.get()); + auto manifests_result = snapshot_cache.Manifests(file_io_); + if (!manifests_result.has_value()) continue; // best-effort + auto manifests = std::move(manifests_result).value(); + for (auto& manifest : manifests) { + valid_manifests.insert(manifest.manifest_path); + + int64_t writer_id = manifest.added_snapshot_id; + bool from_valid_snapshots = valid_ids.contains(writer_id); + bool is_from_ancestor = ancestor_ids.contains(writer_id); + bool is_picked = picked_ancestor_snapshot_ids.contains(writer_id); + if (!from_valid_snapshots && (is_from_ancestor || is_picked) && + manifest.has_deleted_files()) { + manifests_to_scan.insert(std::move(manifest)); + } + } + } + + // Find manifests that were only referenced by snapshots that have expired, + // and split them by what kind of cleanup they need: + // - manifests_to_delete: not referenced by any retained snapshot; + // - manifests_to_scan: from a current-state ancestor and has deleted + // entries (data files now safe to drop); + // - manifests_to_revert: written by an expiring non-ancestor snapshot + // and contains added entries -- those data files were never adopted. + std::unordered_set manifest_lists_to_delete; + manifest_lists_to_delete.reserve(expired_snapshot_ids.size()); + std::unordered_set manifests_to_delete; + manifests_to_delete.reserve(expired_snapshot_ids.size()); + std::unordered_set manifests_to_revert; + manifests_to_revert.reserve(expired_snapshot_ids.size()); + for (const auto& snapshot : metadata_before_expiration.snapshots) { + if (!snapshot) continue; + int64_t snapshot_id = snapshot->snapshot_id; + if (valid_ids.contains(snapshot_id)) continue; + + // Skip cherry-picked snapshots; the picked snapshot owns its cleanup. + if (picked_ancestor_snapshot_ids.contains(snapshot_id)) { + continue; + } + + int64_t source_snapshot_id = -1; + auto src_it = snapshot->summary.find(SnapshotSummaryFields::kSourceSnapshotId); + if (src_it != snapshot->summary.end()) { + auto source_snapshot_id_result = + StringUtils::ParseNumber(src_it->second); + if (!source_snapshot_id_result.has_value()) { + continue; + } + source_snapshot_id = source_snapshot_id_result.value(); + } + // If this commit was cherry-picked from a still-live snapshot, skip it. + if (ancestor_ids.contains(source_snapshot_id) || + picked_ancestor_snapshot_ids.contains(source_snapshot_id)) { + continue; + } + + SnapshotCache snapshot_cache(snapshot.get()); + auto manifests_result = snapshot_cache.Manifests(file_io_); + if (!manifests_result.has_value()) { + continue; + } + + auto manifests = std::move(manifests_result).value(); + for (auto& manifest : manifests) { + if (valid_manifests.contains(manifest.manifest_path)) continue; + manifests_to_delete.insert(manifest.manifest_path); + + int64_t writer_id = manifest.added_snapshot_id; + bool is_from_ancestor = ancestor_ids.contains(writer_id); + bool is_from_expiring_snapshot = expired_snapshot_ids.contains(writer_id); + + if (is_from_ancestor && manifest.has_deleted_files()) { + manifests_to_scan.insert(std::move(manifest)); + } else if (!is_from_ancestor && is_from_expiring_snapshot && + manifest.has_added_files()) { + // The writer must be known-expired so missing history cannot make + // an ancestor look like a reverted snapshot. + manifests_to_revert.insert(std::move(manifest)); + } + } + if (!snapshot->manifest_list.empty()) { + manifest_lists_to_delete.insert(snapshot->manifest_list); + } + } + + // Deleting data files + if (level == CleanupLevel::kAll) { + // Manifests may reference partition specs that were pruned during expiration + // when CleanExpiredMetadata is enabled, so resolve schemas/specs against the + // pre-expiration metadata. + auto files_to_delete = FindFilesToDelete( + metadata_before_expiration, manifests_to_scan, manifests_to_revert, valid_ids); + DeleteFiles(files_to_delete); + } + + // Deleting manifest files + DeleteFiles(manifests_to_delete); + + // Deleting manifest-list files + DeleteFiles(manifest_lists_to_delete); + + // Deleting statistics files + if (HasAnyStatisticsFiles(metadata_before_expiration)) { + DeleteFiles( + StatisticsFilesToDelete(metadata_before_expiration, metadata_after_expiration)); + } + + return {}; + } + + private: + /// \brief Resolve the data files that the incremental pass identified for deletion. + /// + /// For manifests_to_scan, read DELETED entries whose snapshot id is no longer valid. + /// For manifests_to_revert, read every ADDED entry. + std::unordered_set FindFilesToDelete( + const TableMetadata& metadata, + const std::unordered_set& manifests_to_scan, + const std::unordered_set& manifests_to_revert, + const std::unordered_set& valid_ids) { + std::unordered_set files_to_delete; + + for (const auto& manifest : manifests_to_scan) { + auto reader_result = MakeManifestReader(manifest, file_io_, metadata); + if (!reader_result.has_value()) continue; + auto entries_result = reader_result.value()->Entries(); + if (!entries_result.has_value()) continue; + for (const auto& entry : entries_result.value()) { + if (entry.status == ManifestStatus::kDeleted && entry.snapshot_id.has_value() && + !valid_ids.contains(entry.snapshot_id.value()) && entry.data_file) { + files_to_delete.insert(entry.data_file->file_path); + } + } + } + + for (const auto& manifest : manifests_to_revert) { + auto reader_result = MakeManifestReader(manifest, file_io_, metadata); + if (!reader_result.has_value()) continue; + auto entries_result = reader_result.value()->Entries(); + if (!entries_result.has_value()) continue; + for (const auto& entry : entries_result.value()) { + if (entry.status == ManifestStatus::kAdded && entry.data_file) { + files_to_delete.insert(entry.data_file->file_path); + } + } + } + + return files_to_delete; + } +}; + +/// \brief True if any retained snapshot sits outside the current main ancestry. +bool HasNonMainSnapshots(const TableMetadata& metadata) { + auto current_result = metadata.SnapshotById(metadata.current_snapshot_id); + if (!current_result.has_value() || current_result.value() == nullptr) { + return !metadata.snapshots.empty(); + } + auto ancestors_result = SnapshotUtil::AncestorsOf( + current_result.value()->snapshot_id, + [&metadata](int64_t id) { return metadata.SnapshotById(id); }); + if (!ancestors_result.has_value()) { + return true; + } + std::unordered_set main_ancestors; + for (const auto& a : ancestors_result.value()) { + if (a) main_ancestors.insert(a->snapshot_id); + } + for (const auto& snapshot : metadata.snapshots) { + if (snapshot && !main_ancestors.contains(snapshot->snapshot_id)) { + return true; + } + } + return false; +} + +/// \brief True if any expired snapshot lived outside the current main ancestry. +/// +/// When `before` has no current snapshot, the main-ancestor set is empty; any +/// removed snapshot then counts as "non-main" and returns true. This guards the +/// dispatch in Finalize() against picking incremental cleanup when the before-state +/// has snapshots but no current pointer. +bool HasRemovedNonMainAncestors(const TableMetadata& before, const TableMetadata& after) { + std::unordered_set main_ancestors; + auto current_result = before.SnapshotById(before.current_snapshot_id); + if (current_result.has_value() && current_result.value() != nullptr) { + auto ancestors_result = SnapshotUtil::AncestorsOf( + current_result.value()->snapshot_id, + [&before](int64_t id) { return before.SnapshotById(id); }); + if (!ancestors_result.has_value()) { + return true; + } + for (const auto& a : ancestors_result.value()) { + if (a) main_ancestors.insert(a->snapshot_id); + } + } + std::unordered_set after_ids; + after_ids.reserve(after.snapshots.size()); + for (const auto& s : after.snapshots) { + if (s) after_ids.insert(s->snapshot_id); + } + for (const auto& snapshot : before.snapshots) { + if (!snapshot) continue; + bool removed = !after_ids.contains(snapshot->snapshot_id); + bool in_main = main_ancestors.contains(snapshot->snapshot_id); + if (removed && !in_main) { + return true; + } + } + return false; +} + } // namespace Result> ExpireSnapshots::Make( @@ -464,7 +774,7 @@ Result> ExpireSnapshots::UnreferencedSnapshotIdsToRe for (const auto& snapshot : base().snapshots) { ICEBERG_DCHECK(snapshot != nullptr, "Snapshot is null"); if (!referenced_ids.contains(snapshot->snapshot_id) && - snapshot->timestamp_ms > default_expire_older_than_) { + snapshot->timestamp_ms >= default_expire_older_than_) { // unreferenced and not old enough to be expired ids_to_retain.insert(snapshot->snapshot_id); } @@ -528,6 +838,8 @@ Result ExpireSnapshots::Apply() { ICEBERG_PRECHECK(!retained_id_to_refs.contains(id), "Cannot expire {}. Still referenced by refs", id); } + std::unordered_set explicit_snapshot_ids(snapshot_ids_to_expire_.begin(), + snapshot_ids_to_expire_.end()); ICEBERG_ASSIGN_OR_RAISE(auto all_branch_snapshot_ids, ComputeAllBranchSnapshotIdsToRetain(retained_refs)); ICEBERG_ASSIGN_OR_RAISE(auto unreferenced_snapshot_ids, @@ -544,8 +856,10 @@ Result ExpireSnapshots::Apply() { result.refs_to_remove.push_back(key_to_ref.first); } }); - std::ranges::for_each(base.snapshots, [&ids_to_retain, &result](const auto& snapshot) { - if (snapshot && !ids_to_retain.contains(snapshot->snapshot_id)) { + std::ranges::for_each(base.snapshots, [&explicit_snapshot_ids, &ids_to_retain, + &result](const auto& snapshot) { + if (snapshot && (explicit_snapshot_ids.contains(snapshot->snapshot_id) || + !ids_to_retain.contains(snapshot->snapshot_id))) { result.snapshot_ids_to_remove.push_back(snapshot->snapshot_id); } }); @@ -608,16 +922,24 @@ Status ExpireSnapshots::Finalize(Result commit_result) { auto metadata_before_expiration_ptr = apply_result_->metadata_before_expiration; const TableMetadata& metadata_before_expiration = *metadata_before_expiration_ptr; const TableMetadata& metadata_after_expiration = *commit_result.value(); - std::unordered_set expired_ids(apply_result_->snapshot_ids_to_remove.begin(), - apply_result_->snapshot_ids_to_remove.end()); apply_result_.reset(); - // File cleanup is best-effort: log and continue on individual file deletion failures - ReachableFileCleanup strategy(ctx_->table->io(), delete_func_); - return strategy.CleanFiles(metadata_before_expiration, metadata_after_expiration, - expired_ids, cleanup_level_); + // Pick incremental cleanup when the expiration is a simple linear-ancestry walk: + // no explicit snapshot IDs, no removed snapshots outside main ancestry, and no + // retained snapshots outside main ancestry. + const bool can_use_incremental = + !specified_snapshot_id_ && + !HasRemovedNonMainAncestors(metadata_before_expiration, + metadata_after_expiration) && + !HasNonMainSnapshots(metadata_after_expiration); + + if (can_use_incremental) { + return IncrementalFileCleanup(ctx_->table->io(), delete_func_) + .CleanFiles(metadata_before_expiration, metadata_after_expiration, + cleanup_level_); + } + return ReachableFileCleanup(ctx_->table->io(), delete_func_) + .CleanFiles(metadata_before_expiration, metadata_after_expiration, cleanup_level_); } -// TODO(shangxinli): add IncrementalFileCleanup strategy for linear ancestry optimization. - } // namespace iceberg diff --git a/src/iceberg/update/expire_snapshots.h b/src/iceberg/update/expire_snapshots.h index 7c1588aa5..a5b6e3b32 100644 --- a/src/iceberg/update/expire_snapshots.h +++ b/src/iceberg/update/expire_snapshots.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include diff --git a/src/iceberg/update/merging_snapshot_update.cc b/src/iceberg/update/merging_snapshot_update.cc new file mode 100644 index 000000000..50a97c15a --- /dev/null +++ b/src/iceberg/update/merging_snapshot_update.cc @@ -0,0 +1,850 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/update/merging_snapshot_update.h" + +#include +#include +#include + +#include "iceberg/constants.h" +#include "iceberg/delete_file_index.h" +#include "iceberg/expression/expressions.h" +#include "iceberg/expression/inclusive_metrics_evaluator.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/manifest/manifest_list.h" +#include "iceberg/manifest/manifest_reader.h" +#include "iceberg/manifest/manifest_util_internal.h" +#include "iceberg/manifest/manifest_writer.h" +#include "iceberg/partition_spec.h" +#include "iceberg/schema.h" +#include "iceberg/snapshot.h" +#include "iceberg/table.h" +#include "iceberg/table_metadata.h" +#include "iceberg/table_properties.h" +#include "iceberg/transaction.h" +#include "iceberg/util/macros.h" +#include "iceberg/util/snapshot_util_internal.h" + +namespace iceberg { + +MergingSnapshotUpdate::MergingSnapshotUpdate(std::string table_name, + std::shared_ptr ctx) + : SnapshotUpdate(std::move(ctx)), + table_name_(std::move(table_name)), + delete_expression_(Expressions::AlwaysFalse()), + data_filter_manager_(ManifestContent::kData, ctx_->table->io()), + delete_filter_manager_(ManifestContent::kDeletes, ctx_->table->io()), + data_merge_manager_( + base().properties.Get(TableProperties::kManifestTargetSizeBytes), + base().properties.Get(TableProperties::kManifestMinMergeCount), + base().properties.Get(TableProperties::kManifestMergeEnabled)), + delete_merge_manager_( + base().properties.Get(TableProperties::kManifestTargetSizeBytes), + base().properties.Get(TableProperties::kManifestMinMergeCount), + base().properties.Get(TableProperties::kManifestMergeEnabled)) {} + +// ------------------------------------------------------------------------- +// Primitive API +// ------------------------------------------------------------------------- + +Status MergingSnapshotUpdate::AddDataFile(std::shared_ptr file) { + if (!file) { + return InvalidArgument("Cannot add a null data file"); + } + if (!file->partition_spec_id.has_value()) { + return InvalidArgument("Data file must have a partition spec ID"); + } + + int32_t spec_id = file->partition_spec_id.value(); + ICEBERG_ASSIGN_OR_RAISE(auto spec, base().PartitionSpecById(spec_id)); + + // Suppress first_row_id — it will be assigned by the commit, not inherited from the + // source file. + file->first_row_id = std::nullopt; + + auto& data_files = new_data_files_by_spec_[spec_id]; + auto [it, inserted] = data_files.insert(file); + if (inserted) { + has_new_data_files_ = true; + ICEBERG_RETURN_UNEXPECTED(added_data_files_summary_.AddedFile(*spec, *file)); + } + return {}; +} + +Status MergingSnapshotUpdate::ValidateNewDeleteFile(const DataFile& file) { + if (file.content == DataFile::Content::kData) { + return InvalidArgument("Expected a delete file but got a data file: {}", + file.file_path); + } + const int8_t format_version = base().format_version; + const bool is_dv = file.referenced_data_file.has_value(); + switch (format_version) { + case 1: + return InvalidArgument("Deletes are supported in V2 and above"); + case 2: + // Position deletes must NOT be DVs in v2. + if (file.content == DataFile::Content::kPositionDeletes && is_dv) { + return InvalidArgument("Must not use DVs for position deletes in V2: {}", + file.file_path); + } + break; + default: + if (format_version >= 3) { + // Position deletes MUST be DVs in v3+. + if (file.content == DataFile::Content::kPositionDeletes && !is_dv) { + return InvalidArgument("Must use DVs for position deletes in V{}: {}", + format_version, file.file_path); + } + } else { + return InvalidArgument("Unsupported format version: {}", format_version); + } + break; + } + return {}; +} + +Status MergingSnapshotUpdate::AddDeleteFile(std::shared_ptr file) { + if (!file) { + return InvalidArgument("Cannot add a null delete file"); + } + ICEBERG_RETURN_UNEXPECTED(ValidateNewDeleteFile(*file)); + if (!file->partition_spec_id.has_value()) { + return InvalidArgument("Delete file must have a partition spec ID"); + } + ICEBERG_ASSIGN_OR_RAISE(auto spec, + base().PartitionSpecById(file->partition_spec_id.value())); + ICEBERG_RETURN_UNEXPECTED(added_delete_files_summary_.AddedFile(*spec, *file)); + has_new_delete_files_ = true; + new_delete_files_.push_back(std::move(file)); + return {}; +} + +Status MergingSnapshotUpdate::DeleteDataFile(std::shared_ptr file) { + if (!file) { + return InvalidArgument("Cannot delete a null data file"); + } + return data_filter_manager_.DeleteFile(std::move(file)); +} + +Status MergingSnapshotUpdate::DeleteDeleteFile(std::shared_ptr file) { + if (!file) { + return InvalidArgument("Cannot delete a null delete file"); + } + return delete_filter_manager_.DeleteFile(std::move(file)); +} + +void MergingSnapshotUpdate::DeleteByPath(std::string_view path) { + data_filter_manager_.DeleteFile(path); +} + +Status MergingSnapshotUpdate::DeleteByRowFilter(std::shared_ptr expr) { + // If a delete file matches the row filter, it can also be removed because the rows + // it references will also be deleted. Both filter managers receive the expression. + delete_expression_ = expr; + ICEBERG_RETURN_UNEXPECTED(data_filter_manager_.DeleteByRowFilter(expr)); + return delete_filter_manager_.DeleteByRowFilter(std::move(expr)); +} + +void MergingSnapshotUpdate::DropPartition(int32_t spec_id, PartitionValues partition) { + // Dropping data in a partition also drops all delete files in that partition. + data_filter_manager_.DropPartition(spec_id, partition); + delete_filter_manager_.DropPartition(spec_id, std::move(partition)); +} + +void MergingSnapshotUpdate::FailMissingDeletePaths() { + data_filter_manager_.FailMissingDeletePaths(); + delete_filter_manager_.FailMissingDeletePaths(); +} + +void MergingSnapshotUpdate::FailAnyDelete() { + data_filter_manager_.FailAnyDelete(); + delete_filter_manager_.FailAnyDelete(); +} + +void MergingSnapshotUpdate::SetNewDataFilesDataSequenceNumber(int64_t sequence_number) { + new_data_files_data_seq_number_ = sequence_number; +} + +void MergingSnapshotUpdate::CaseSensitive(bool case_sensitive) { + case_sensitive_ = case_sensitive; + data_filter_manager_.CaseSensitive(case_sensitive); + delete_filter_manager_.CaseSensitive(case_sensitive); +} + +void MergingSnapshotUpdate::Set(const std::string& property, const std::string& value) { + summary_builder().Set(property, value); +} + +Result> MergingSnapshotUpdate::DataSpec() const { + if (new_data_files_by_spec_.size() != 1) { + return InvalidArgument("DataSpec() requires exactly one partition spec; got {}", + new_data_files_by_spec_.size()); + } + return base().PartitionSpecById(new_data_files_by_spec_.begin()->first); +} + +std::vector> MergingSnapshotUpdate::AddedDataFiles() const { + std::vector> result; + for (const auto& [spec_id, files] : new_data_files_by_spec_) { + for (const auto& file : files) { + result.push_back(file); + } + } + return result; +} + +Status MergingSnapshotUpdate::AddDeleteFile(std::shared_ptr /*file*/, + int64_t /*data_sequence_number*/) { + return NotImplemented( + "AddDeleteFile with explicit data sequence number is not yet implemented"); +} + +Status MergingSnapshotUpdate::AddManifest(ManifestFile manifest) { + if (manifest.content != ManifestContent::kData) { + return InvalidArgument("Cannot append delete manifest: {}", manifest.manifest_path); + } + if (manifest.has_existing_files()) { + return InvalidArgument("Cannot append manifest with existing files: {}", + manifest.manifest_path); + } + if (manifest.has_deleted_files()) { + return InvalidArgument("Cannot append manifest with deleted files: {}", + manifest.manifest_path); + } + if (manifest.added_snapshot_id != kInvalidSnapshotId) { + return InvalidArgument("Snapshot id must be assigned during commit: {}", + manifest.manifest_path); + } + if (manifest.first_row_id.has_value()) { + return InvalidArgument("Cannot append manifest with assigned first_row_id: {}", + manifest.manifest_path); + } + + if (can_inherit_snapshot_id()) { + appended_manifests_summary_.AddedManifest(manifest); + append_manifests_.push_back(std::move(manifest)); + } else { + ICEBERG_ASSIGN_OR_RAISE(auto copied, CopyManifest(manifest)); + rewritten_append_manifests_.push_back(std::move(copied)); + } + return {}; +} + +Result MergingSnapshotUpdate::CopyManifest(const ManifestFile& manifest) { + const TableMetadata& current = base(); + ICEBERG_ASSIGN_OR_RAISE(auto schema, current.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto spec, + current.PartitionSpecById(manifest.partition_spec_id)); + std::string path = ManifestPath(); + all_written_manifests_.insert(path); + return CopyAppendManifest(manifest, ctx_->table->io(), schema, spec, SnapshotId(), path, + current.format_version, &appended_manifests_summary_); +} + +// ------------------------------------------------------------------------- +// State queries +// ------------------------------------------------------------------------- + +bool MergingSnapshotUpdate::AddsDataFiles() const { + return !new_data_files_by_spec_.empty(); +} + +bool MergingSnapshotUpdate::AddsDeleteFiles() const { return !new_delete_files_.empty(); } + +bool MergingSnapshotUpdate::DeletesDataFiles() const { + return data_filter_manager_.ContainsDeletes(); +} + +bool MergingSnapshotUpdate::DeletesDeleteFiles() const { + return delete_filter_manager_.ContainsDeletes(); +} + +// ------------------------------------------------------------------------- +// Apply pipeline +// ------------------------------------------------------------------------- + +ManifestWriterFactory MergingSnapshotUpdate::MakeTrackedWriterFactory() { + return [this](int32_t spec_id, + ManifestContent content) -> Result> { + const TableMetadata& meta = base(); + ICEBERG_ASSIGN_OR_RAISE(auto schema, meta.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto spec, meta.PartitionSpecById(spec_id)); + std::string path = ManifestPath(); + all_written_manifests_.insert(path); + return ManifestWriter::MakeWriter(meta.format_version, SnapshotId(), std::move(path), + ctx_->table->io(), std::move(spec), + std::move(schema), content); + }; +} + +Result> MergingSnapshotUpdate::WriteNewDataManifests() { + // If new files were staged after the cache was populated (commit retry), invalidate. + if (has_new_data_files_ && cached_new_data_manifests_.has_value()) { + for (const auto& m : *cached_new_data_manifests_) { + std::ignore = DeleteFile(m.manifest_path); + } + cached_new_data_manifests_.reset(); + } + + if (cached_new_data_manifests_.has_value()) { + return *cached_new_data_manifests_; + } + + std::vector result; + for (const auto& [spec_id, data_files] : new_data_files_by_spec_) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, base().PartitionSpecById(spec_id)); + ICEBERG_ASSIGN_OR_RAISE( + auto written, + WriteDataManifests(data_files.as_span(), spec, new_data_files_data_seq_number_)); + for (const auto& m : written) { + all_written_manifests_.insert(m.manifest_path); + } + result.insert(result.end(), std::make_move_iterator(written.begin()), + std::make_move_iterator(written.end())); + } + + cached_new_data_manifests_ = result; + has_new_data_files_ = false; + return result; +} + +Result> MergingSnapshotUpdate::WriteNewDeleteManifests() { + // If new files were staged after the cache was populated (commit retry), invalidate. + if (has_new_delete_files_ && cached_new_delete_manifests_.has_value()) { + for (const auto& m : *cached_new_delete_manifests_) { + std::ignore = DeleteFile(m.manifest_path); + } + cached_new_delete_manifests_.reset(); + } + + if (cached_new_delete_manifests_.has_value()) { + return *cached_new_delete_manifests_; + } + + std::vector result; + if (!new_delete_files_.empty()) { + ICEBERG_ASSIGN_OR_RAISE(auto spec, base().PartitionSpec()); + ICEBERG_ASSIGN_OR_RAISE(auto written, + WriteDeleteManifests(std::span(new_delete_files_), spec)); + for (const auto& m : written) { + all_written_manifests_.insert(m.manifest_path); + } + result.insert(result.end(), std::make_move_iterator(written.begin()), + std::make_move_iterator(written.end())); + } + + cached_new_delete_manifests_ = result; + has_new_delete_files_ = false; + return result; +} + +Result> MergingSnapshotUpdate::Apply( + const TableMetadata& metadata_to_update, const std::shared_ptr& snapshot) { + // Re-validate buffered delete files against the current format version. A format + // upgrade between staging and commit could make previously-valid files invalid. + for (const auto& file : new_delete_files_) { + ICEBERG_RETURN_UNEXPECTED(ValidateNewDeleteFile(*file)); + } + + // Rebuild summary from stable sub-builders so that commit retries don't double-count. + summary_builder().Clear(); + summary_builder().Merge(added_data_files_summary_); + summary_builder().Merge(added_delete_files_summary_); + summary_builder().Merge(appended_manifests_summary_); + + auto tracked_factory = MakeTrackedWriterFactory(); + + // Step 1: Filter data manifests. + ICEBERG_ASSIGN_OR_RAISE(auto filtered_data, + data_filter_manager_.FilterManifests( + metadata_to_update, snapshot, tracked_factory)); + + // Track deleted data files in the summary builder. + for (const auto& file : data_filter_manager_.FilesToBeDeleted()) { + if (!file->partition_spec_id.has_value()) { + continue; + } + ICEBERG_ASSIGN_OR_RAISE( + auto spec, metadata_to_update.PartitionSpecById(*file->partition_spec_id)); + ICEBERG_RETURN_UNEXPECTED(summary_builder().DeletedFile(*spec, *file)); + } + + // Step 2: Compute min data sequence number; set up delete filter cleanup. + // Use last_sequence_number as the initial value so that an empty filtered list + // produces a sensible minimum. Skip manifests with kUnassignedSequenceNumber — + // those are rewritten manifests whose sequence number will be assigned at commit time. + int64_t min_data_seq = metadata_to_update.last_sequence_number; + for (const auto& manifest : filtered_data) { + if (manifest.min_sequence_number != kUnassignedSequenceNumber) { + min_data_seq = std::min(min_data_seq, manifest.min_sequence_number); + } + } + delete_filter_manager_.DropDeleteFilesOlderThan(min_data_seq); + delete_filter_manager_.RemoveDanglingDeletesFor( + data_filter_manager_.FilesToBeDeleted()); + + // Step 3: Filter delete manifests. + ICEBERG_ASSIGN_OR_RAISE(auto filtered_deletes, + delete_filter_manager_.FilterManifests( + metadata_to_update, snapshot, tracked_factory)); + + // Track deleted delete files in the summary builder. + for (const auto& file : delete_filter_manager_.FilesToBeDeleted()) { + if (!file->partition_spec_id.has_value()) { + continue; + } + ICEBERG_ASSIGN_OR_RAISE( + auto spec, metadata_to_update.PartitionSpecById(*file->partition_spec_id)); + ICEBERG_RETURN_UNEXPECTED(summary_builder().DeletedFile(*spec, *file)); + } + + // Drop manifests with no live files — they carry no data and should not be merged + // into the new snapshot. + int64_t snapshot_id = SnapshotId(); + auto should_keep = [snapshot_id](const ManifestFile& m) { + return m.has_added_files() || m.has_existing_files() || + m.added_snapshot_id == snapshot_id; + }; + std::erase_if(filtered_data, [&](const ManifestFile& m) { return !should_keep(m); }); + std::erase_if(filtered_deletes, [&](const ManifestFile& m) { return !should_keep(m); }); + + // Step 4: Write (or retrieve cached) new data manifests. + ICEBERG_ASSIGN_OR_RAISE(auto written_data_manifests, WriteNewDataManifests()); + + // Incorporate append manifests (from AddManifest), stamping each with the + // current snapshot ID. append_manifests_ are used directly (inherit path); + // rewritten_append_manifests_ were already copied with the snapshot ID. + std::vector new_data_manifests = std::move(written_data_manifests); + for (auto m : append_manifests_) { + m.added_snapshot_id = snapshot_id; + new_data_manifests.push_back(std::move(m)); + } + for (auto m : rewritten_append_manifests_) { + m.added_snapshot_id = snapshot_id; + new_data_manifests.push_back(std::move(m)); + } + + // Step 5: Write (or retrieve cached) new delete manifests. + ICEBERG_ASSIGN_OR_RAISE(auto new_delete_manifests, WriteNewDeleteManifests()); + + // Step 6: Merge data manifests. + ICEBERG_ASSIGN_OR_RAISE(auto merged_data, + data_merge_manager_.MergeManifests( + filtered_data, new_data_manifests, SnapshotId(), + metadata_to_update, ctx_->table->io(), tracked_factory)); + + // Step 7: Merge delete manifests. + ICEBERG_ASSIGN_OR_RAISE(auto merged_deletes, + delete_merge_manager_.MergeManifests( + filtered_deletes, new_delete_manifests, SnapshotId(), + metadata_to_update, ctx_->table->io(), tracked_factory)); + + std::vector result; + result.reserve(merged_data.size() + merged_deletes.size()); + result.insert(result.end(), std::make_move_iterator(merged_data.begin()), + std::make_move_iterator(merged_data.end())); + result.insert(result.end(), std::make_move_iterator(merged_deletes.begin()), + std::make_move_iterator(merged_deletes.end())); + + // Manifest count summary. + int32_t manifests_created = 0; + int32_t manifests_kept = 0; + for (const auto& m : result) { + if (m.added_snapshot_id == snapshot_id) { + ++manifests_created; + } else { + ++manifests_kept; + } + } + int32_t replaced_manifests_count = data_filter_manager_.ReplacedManifestsCount() + + delete_filter_manager_.ReplacedManifestsCount() + + data_merge_manager_.ReplacedManifestsCount() + + delete_merge_manager_.ReplacedManifestsCount(); + summary_builder().SetManifestCounts(manifests_created, manifests_kept, + replaced_manifests_count); + + return result; +} + +void MergingSnapshotUpdate::CleanUncommitted( + const std::unordered_set& committed) { + for (const auto& path : all_written_manifests_) { + if (!committed.contains(path)) { + std::ignore = DeleteFile(path); + } + } + all_written_manifests_.clear(); + cached_new_data_manifests_.reset(); + cached_new_delete_manifests_.reset(); + has_new_data_files_ = false; + has_new_delete_files_ = false; + + // rewritten_append_manifests_ are always owned by the table (copied by us), + // so delete any that were not committed. + for (const auto& m : rewritten_append_manifests_) { + if (!committed.contains(m.manifest_path)) { + std::ignore = DeleteFile(m.manifest_path); + } + } + + // append_manifests_ are only owned by the table if the commit succeeded + // (i.e., at least one manifest was committed). + if (!committed.empty()) { + for (const auto& m : append_manifests_) { + if (!committed.contains(m.manifest_path)) { + std::ignore = DeleteFile(m.manifest_path); + } + } + } +} + +std::unordered_map MergingSnapshotUpdate::Summary() { + summary_builder().SetPartitionSummaryLimit( + base().properties.Get(TableProperties::kWritePartitionSummaryLimit)); + return summary_builder().Build(); +} + +// ------------------------------------------------------------------------- +// Conflict-detection helpers +// ------------------------------------------------------------------------- + +Status MergingSnapshotUpdate::ValidateAddedDataFiles( + const TableMetadata& metadata, int64_t starting_snapshot_id, + std::shared_ptr filter, const std::shared_ptr& parent, + std::shared_ptr io, bool case_sensitive) { + if (parent == nullptr) { + return {}; + } + + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto ancestors, + SnapshotUtil::AncestorsBetween(metadata, parent->snapshot_id, + starting_snapshot_id)); + + // Only scan manifests from APPEND and OVERWRITE snapshots — those are the only + // operations that add data files. + std::unordered_set matching_snapshot_ids; + for (const auto& snap : ancestors) { + auto op = snap->Operation(); + if (op == DataOperation::kAppend || op == DataOperation::kOverwrite) { + matching_snapshot_ids.insert(snap->snapshot_id); + } + } + + std::unique_ptr evaluator; + if (filter != nullptr) { + ICEBERG_ASSIGN_OR_RAISE( + evaluator, InclusiveMetricsEvaluator::Make(filter, *schema, case_sensitive)); + } + + for (const auto& snapshot : ancestors) { + if (!matching_snapshot_ids.contains(snapshot->snapshot_id)) { + continue; + } + auto cached = SnapshotCache(snapshot.get()); + ICEBERG_ASSIGN_OR_RAISE(auto data_manifests, cached.DataManifests(io)); + + for (const auto& manifest : data_manifests) { + if (!matching_snapshot_ids.contains(manifest.added_snapshot_id)) { + continue; + } + ICEBERG_ASSIGN_OR_RAISE(auto spec, + metadata.PartitionSpecById(manifest.partition_spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(manifest, io, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + + for (const auto& entry : entries) { + if (entry.status != ManifestStatus::kAdded) { + continue; + } + if (entry.data_file == nullptr) { + continue; + } + if (evaluator != nullptr) { + ICEBERG_ASSIGN_OR_RAISE(bool matches, evaluator->Evaluate(*entry.data_file)); + if (!matches) { + continue; + } + } + return InvalidArgument( + "Found conflicting files that can contain rows matching {}:" + " {} in snapshot {}", + filter != nullptr ? filter->ToString() : "any expression", + entry.data_file->file_path, snapshot->snapshot_id); + } + } + } + return {}; +} + +Status MergingSnapshotUpdate::ValidateDataFilesExist( + const TableMetadata& metadata, int64_t starting_snapshot_id, + const std::unordered_set& file_paths, bool allow_deletes, + std::shared_ptr filter, const std::shared_ptr& parent, + std::shared_ptr io, bool case_sensitive) { + if (parent == nullptr || file_paths.empty()) { + return {}; + } + + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + ICEBERG_ASSIGN_OR_RAISE(auto ancestors, + SnapshotUtil::AncestorsBetween(metadata, parent->snapshot_id, + starting_snapshot_id)); + + // Build the set of snapshot IDs to scan: OVERWRITE and REPLACE always included; + // DELETE included only when allow_deletes is false. + std::unordered_set matching_snapshot_ids; + for (const auto& snap : ancestors) { + auto op = snap->Operation(); + if (!op.has_value()) { + continue; + } + if (*op == DataOperation::kOverwrite || *op == DataOperation::kReplace) { + matching_snapshot_ids.insert(snap->snapshot_id); + } else if (!allow_deletes && *op == DataOperation::kDelete) { + matching_snapshot_ids.insert(snap->snapshot_id); + } + } + + // Build a metrics evaluator for the conflict-detection filter, if provided. + std::unique_ptr evaluator; + if (filter != nullptr) { + ICEBERG_ASSIGN_OR_RAISE( + evaluator, InclusiveMetricsEvaluator::Make(filter, *schema, case_sensitive)); + } + + for (const auto& snapshot : ancestors) { + if (!matching_snapshot_ids.contains(snapshot->snapshot_id)) { + continue; + } + auto cached = SnapshotCache(snapshot.get()); + ICEBERG_ASSIGN_OR_RAISE(auto data_manifests, cached.DataManifests(io)); + + for (const auto& manifest : data_manifests) { + if (!matching_snapshot_ids.contains(manifest.added_snapshot_id)) { + continue; + } + ICEBERG_ASSIGN_OR_RAISE(auto spec, + metadata.PartitionSpecById(manifest.partition_spec_id)); + ICEBERG_ASSIGN_OR_RAISE(auto reader, + ManifestReader::Make(manifest, io, schema, spec)); + ICEBERG_ASSIGN_OR_RAISE(auto entries, reader->Entries()); + + for (const auto& entry : entries) { + if (entry.status != ManifestStatus::kDeleted) { + continue; + } + if (entry.data_file == nullptr) { + continue; + } + if (!file_paths.contains(entry.data_file->file_path)) { + continue; + } + if (evaluator != nullptr) { + ICEBERG_ASSIGN_OR_RAISE(bool matches, evaluator->Evaluate(*entry.data_file)); + if (!matches) { + continue; + } + } + return InvalidArgument("Cannot commit, missing data files: {} in snapshot {}", + entry.data_file->file_path, snapshot->snapshot_id); + } + } + } + return {}; +} + +Status MergingSnapshotUpdate::ValidateNoNewDeletesForDataFiles( + const TableMetadata& metadata, int64_t starting_snapshot_id, + const DataFileSet& replaced_files, const std::shared_ptr& parent, + std::shared_ptr io, bool ignore_equality_deletes) { + if (parent == nullptr || replaced_files.empty() || metadata.format_version < 2) { + return {}; + } + + // Build an index of delete files added since starting_snapshot_id. + // Covers both position and equality deletes; the caller controls whether + // equality deletes are ignored. + ICEBERG_ASSIGN_OR_RAISE(auto deletes, AddedDeleteFiles(metadata, starting_snapshot_id, + nullptr, nullptr, parent, io)); + + if (deletes->empty()) { + return {}; + } + + // Compute the starting sequence number for the data file check. + int64_t starting_seq = TableMetadata::kInitialSequenceNumber; + if (auto snap_result = metadata.SnapshotById(starting_snapshot_id); + snap_result.has_value()) { + starting_seq = snap_result.value()->sequence_number; + } + + for (const auto& data_file : replaced_files) { + ICEBERG_ASSIGN_OR_RAISE(auto delete_files, + deletes->ForDataFile(starting_seq, *data_file)); + if (ignore_equality_deletes) { + // Only fail on position deletes — equality deletes at higher sequence numbers + // still apply to the rewritten files and are not a conflict. + for (const auto& df : delete_files) { + if (df->content == DataFile::Content::kPositionDeletes) { + return InvalidArgument( + "Cannot commit, found new position delete for replaced data file: {}", + data_file->file_path); + } + } + } else { + if (!delete_files.empty()) { + return InvalidArgument( + "Cannot commit, found new delete for replaced data file: {}", + data_file->file_path); + } + } + } + return {}; +} + +Status MergingSnapshotUpdate::ValidateAddedDataFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + const PartitionSet& /*partition_set*/, const std::shared_ptr& /*parent*/, + std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateAddedDataFiles with PartitionSet is not yet implemented"); +} + +Status MergingSnapshotUpdate::ValidateNoNewDeletesForDataFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + std::shared_ptr /*data_filter*/, const DataFileSet& /*replaced_files*/, + const std::shared_ptr& /*parent*/, std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateNoNewDeletesForDataFiles with data filter is not yet implemented"); +} + +Status MergingSnapshotUpdate::ValidateNoNewDeleteFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + std::shared_ptr /*data_filter*/, + const std::shared_ptr& /*parent*/, std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateNoNewDeleteFiles with Expression is not yet implemented"); +} + +Status MergingSnapshotUpdate::ValidateNoNewDeleteFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + const PartitionSet& /*partition_set*/, const std::shared_ptr& /*parent*/, + std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateNoNewDeleteFiles with PartitionSet is not yet implemented"); +} + +Status MergingSnapshotUpdate::ValidateDeletedDataFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + std::shared_ptr /*data_filter*/, + const std::shared_ptr& /*parent*/, std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateDeletedDataFiles with Expression is not yet implemented"); +} + +Status MergingSnapshotUpdate::ValidateDeletedDataFiles( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + const PartitionSet& /*partition_set*/, const std::shared_ptr& /*parent*/, + std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateDeletedDataFiles with PartitionSet is not yet implemented"); +} + +Result> MergingSnapshotUpdate::AddedDeleteFiles( + const TableMetadata& metadata, int64_t starting_snapshot_id, + std::shared_ptr data_filter, std::shared_ptr partition_set, + const std::shared_ptr& parent, std::shared_ptr io, + bool case_sensitive) { + ICEBERG_ASSIGN_OR_RAISE(auto schema, metadata.Schema()); + + if (parent == nullptr || metadata.format_version < 2) { + ICEBERG_ASSIGN_OR_RAISE(auto specs_ref, + TableMetadataCache(&metadata).GetPartitionSpecsById()); + std::unordered_map> empty_specs( + specs_ref.get().begin(), specs_ref.get().end()); + ICEBERG_ASSIGN_OR_RAISE(auto builder, DeleteFileIndex::BuilderFor( + io, schema, std::move(empty_specs), {})); + return builder.Build(); + } + + ICEBERG_ASSIGN_OR_RAISE(auto ancestors, + SnapshotUtil::AncestorsBetween(metadata, parent->snapshot_id, + starting_snapshot_id)); + + // Collect delete manifests from OVERWRITE and DELETE snapshots only. + std::unordered_set matching_snapshot_ids; + for (const auto& snap : ancestors) { + auto op = snap->Operation(); + if (op == DataOperation::kOverwrite || op == DataOperation::kDelete) { + matching_snapshot_ids.insert(snap->snapshot_id); + } + } + + std::vector delete_manifests; + for (const auto& snapshot : ancestors) { + if (!matching_snapshot_ids.contains(snapshot->snapshot_id)) { + continue; + } + auto cached = SnapshotCache(snapshot.get()); + ICEBERG_ASSIGN_OR_RAISE(auto manifests, cached.DeleteManifests(io)); + for (const auto& m : manifests) { + if (matching_snapshot_ids.contains(m.added_snapshot_id)) { + delete_manifests.push_back(m); + } + } + } + + // Compute the starting sequence number from the starting snapshot. + int64_t starting_seq = TableMetadata::kInitialSequenceNumber; + if (auto snap_result = metadata.SnapshotById(starting_snapshot_id); + snap_result.has_value()) { + starting_seq = snap_result.value()->sequence_number; + } + + ICEBERG_ASSIGN_OR_RAISE(auto specs_ref, + TableMetadataCache(&metadata).GetPartitionSpecsById()); + std::unordered_map> specs_by_id( + specs_ref.get().begin(), specs_ref.get().end()); + + ICEBERG_ASSIGN_OR_RAISE(auto builder, + DeleteFileIndex::BuilderFor(io, schema, std::move(specs_by_id), + std::move(delete_manifests))); + builder.AfterSequenceNumber(starting_seq); + builder.CaseSensitive(case_sensitive); + if (data_filter != nullptr) { + builder.DataFilter(std::move(data_filter)); + } + if (partition_set != nullptr) { + builder.FilterPartitions(std::move(partition_set)); + } + return builder.Build(); +} + +Status MergingSnapshotUpdate::ValidateAddedDVs( + const TableMetadata& /*metadata*/, int64_t /*starting_snapshot_id*/, + std::shared_ptr /*conflict_filter*/, + const std::shared_ptr& /*parent*/, std::shared_ptr /*io*/) { + return NotImplemented( + "ValidateAddedDVs is not yet supported (deletion vectors require format v3)"); +} + +} // namespace iceberg diff --git a/src/iceberg/update/merging_snapshot_update.h b/src/iceberg/update/merging_snapshot_update.h new file mode 100644 index 000000000..4900d920d --- /dev/null +++ b/src/iceberg/update/merging_snapshot_update.h @@ -0,0 +1,360 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +/// \file iceberg/update/merging_snapshot_update.h + +#include +#include +#include +#include +#include +#include +#include + +#include "iceberg/delete_file_index.h" +#include "iceberg/iceberg_export.h" +#include "iceberg/manifest/manifest_filter_manager.h" +#include "iceberg/manifest/manifest_merge_manager.h" +#include "iceberg/result.h" +#include "iceberg/type_fwd.h" +#include "iceberg/update/snapshot_update.h" +#include "iceberg/util/data_file_set.h" + +namespace iceberg { + +/// \brief Abstract base class for all merge-based snapshot write operations. +/// +/// Provides the complete filter → write → merge pipeline that all merge-based +/// operations (MergeAppend, OverwriteFiles, RowDelta, ReplacePartitions, +/// RewriteFiles) share. Subclasses only need to implement `operation()` and +/// call the protected primitive API to describe what changes to make. +/// +/// The Apply() pipeline: +/// 1. Filter data manifests (via data_filter_manager_) +/// 2. Compute min data sequence number and set up delete filter cleanup +/// 3. Filter delete manifests (via delete_filter_manager_) +/// 4. Write new data manifests (cached for commit retry) +/// 5. Write new delete manifests (cached for commit retry) +/// 6. Merge data manifests (via data_merge_manager_) +/// 7. Merge delete manifests (via delete_merge_manager_) +/// +/// See also: org.apache.iceberg.MergingSnapshotProducer +class ICEBERG_EXPORT MergingSnapshotUpdate : public SnapshotUpdate { + public: + ~MergingSnapshotUpdate() override = default; + + // SnapshotUpdate overrides + Result> Apply( + const TableMetadata& metadata_to_update, + const std::shared_ptr& snapshot) override; + + void CleanUncommitted(const std::unordered_set& committed) override; + + std::unordered_map Summary() override; + + /// \brief Set a custom property in the snapshot summary. + void Set(const std::string& property, const std::string& value); + + protected: + /// \brief Constructor; reads merge configuration from table properties. + explicit MergingSnapshotUpdate(std::string table_name, + std::shared_ptr ctx); + + // ------------------------------------------------------------------------- + // Primitive API — called by subclasses to describe the desired changes + // ------------------------------------------------------------------------- + + /// \brief Stage a data file to be added to the table. + Status AddDataFile(std::shared_ptr file); + + /// \brief Stage a delete file to be added to the table. + Status AddDeleteFile(std::shared_ptr file); + + /// \brief Validate a delete file against the table format version rules. + /// + /// - Format v1: deletes are not supported. + /// - Format v2: position deletes must NOT be deletion vectors (DVs). + /// - Format v3+: position deletes MUST be deletion vectors (DVs). + Status ValidateNewDeleteFile(const DataFile& file); + + /// \brief Stage a delete file with an explicit data sequence number. + /// + /// \note Not yet implemented; returns NotImplemented error. + Status AddDeleteFile(std::shared_ptr file, int64_t data_sequence_number); + + /// \brief Add all files in a pre-existing data manifest to the new snapshot. + /// + /// The manifest must contain only DATA content and only ADDED entries (no + /// existing or deleted files). If snapshot ID inheritance is enabled and the + /// manifest has no snapshot ID assigned, it is used directly; otherwise it is + /// copied with the current snapshot ID. + Status AddManifest(ManifestFile manifest); + + /// \brief Register a data file (by object) to be deleted from the table. + Status DeleteDataFile(std::shared_ptr file); + + /// \brief Register a delete file (by object) to be removed from the table. + Status DeleteDeleteFile(std::shared_ptr file); + + /// \brief Register a file path to be deleted from the table. + void DeleteByPath(std::string_view path); + + /// \brief Register an expression to delete matching rows. + /// + /// Both data and delete filter managers receive the expression: delete files that + /// match the row filter can also be removed because those rows will be deleted. + Status DeleteByRowFilter(std::shared_ptr expr); + + /// \brief Register a partition to be dropped. + /// + /// Both data and delete filter managers receive the partition drop, since dropping + /// data in a partition also drops all delete files in that partition. + void DropPartition(int32_t spec_id, PartitionValues partition); + + /// \brief Fail if any registered delete path is not found in any manifest. + void FailMissingDeletePaths(); + + /// \brief Fail if any manifest entry matches a delete condition. + void FailAnyDelete(); + + /// \brief Override the data sequence number assigned to all newly-added data files. + void SetNewDataFilesDataSequenceNumber(int64_t sequence_number); + + /// \brief Set case sensitivity for row filter and expression evaluation. + void CaseSensitive(bool case_sensitive); + + // ------------------------------------------------------------------------- + // State queries + // ------------------------------------------------------------------------- + + /// \brief Returns true if case-sensitive matching is enabled (default: true). + bool IsCaseSensitive() const { return case_sensitive_; } + + /// \brief Returns true if any data files have been staged for addition. + bool AddsDataFiles() const; + + /// \brief Returns true if any delete files have been staged for addition. + bool AddsDeleteFiles() const; + + /// \brief Returns true if any data files have been registered for deletion. + bool DeletesDataFiles() const; + + /// \brief Returns true if any delete files have been registered for removal. + bool DeletesDeleteFiles() const; + + /// \brief Returns the row-filter expression set via DeleteByRowFilter, or nullptr. + const std::shared_ptr& RowFilter() const { return delete_expression_; } + + /// \brief Returns the single partition spec for all staged data files. + /// + /// Precondition: exactly one partition spec ID must be represented among staged + /// data files. + Result> DataSpec() const; + + /// \brief Returns all data files staged for addition. + std::vector> AddedDataFiles() const; + + // ------------------------------------------------------------------------- + // Conflict-detection helpers + // ------------------------------------------------------------------------- + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a data file matching the given filter expression. + static Status ValidateAddedDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + std::shared_ptr filter, + const std::shared_ptr& parent, + std::shared_ptr io, + bool case_sensitive = true); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a data file in any partition of the given partition set. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateAddedDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + const PartitionSet& partition_set, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// removed a file whose path is in file_paths (and allow_deletes is false). + static Status ValidateDataFilesExist( + const TableMetadata& metadata, int64_t starting_snapshot_id, + const std::unordered_set& file_paths, bool allow_deletes, + std::shared_ptr filter, const std::shared_ptr& parent, + std::shared_ptr io, bool case_sensitive = true); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a delete file that covers a file in replaced_files. + /// + /// Whether equality deletes are checked is derived automatically from whether + /// a custom data sequence number was set via SetNewDataFilesDataSequenceNumber(): + /// if set, equality deletes are ignored because they still apply to the rewritten + /// files and are not a conflict. + /// + /// Subclasses should prefer this overload over the static one. + Status ValidateNoNewDeletesForDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + const DataFileSet& replaced_files, + const std::shared_ptr& parent, + std::shared_ptr io) const { + return ValidateNoNewDeletesForDataFiles(metadata, starting_snapshot_id, + replaced_files, parent, io, + new_data_files_data_seq_number_.has_value()); + } + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a delete file that covers a file in replaced_files. + /// + /// \param ignore_equality_deletes If true, only position deletes are checked. + /// Set to true when replaced data files have the same sequence number as the + /// new files (e.g. RewriteFiles), so equality deletes at higher sequence numbers + /// still apply and are not a conflict. + static Status ValidateNoNewDeletesForDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + const DataFileSet& replaced_files, + const std::shared_ptr& parent, + std::shared_ptr io, + bool ignore_equality_deletes = false); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a delete file matching the data filter that covers a file in replaced_files. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateNoNewDeletesForDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + std::shared_ptr data_filter, + const DataFileSet& replaced_files, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a delete file matching the given row filter. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateNoNewDeleteFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + std::shared_ptr data_filter, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a delete file matching any partition in the given partition set. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateNoNewDeleteFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + const PartitionSet& partition_set, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// deleted a data file matching the given row filter. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateDeletedDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + std::shared_ptr data_filter, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// deleted a data file in any partition of the given partition set. + /// + /// \note Not yet implemented; returns NotImplemented error. + static Status ValidateDeletedDataFiles(const TableMetadata& metadata, + int64_t starting_snapshot_id, + const PartitionSet& partition_set, + const std::shared_ptr& parent, + std::shared_ptr io); + + /// \brief Build a DeleteFileIndex of delete files added since starting_snapshot_id. + static Result> AddedDeleteFiles( + const TableMetadata& metadata, int64_t starting_snapshot_id, + std::shared_ptr data_filter, + std::shared_ptr partition_set, + const std::shared_ptr& parent, std::shared_ptr io, + bool case_sensitive = true); + + /// \brief Return an error if any snapshot in [starting_snapshot_id+1, parent] + /// added a deletion vector that conflicts with DVs being written. + /// + /// \note Deletion vectors (format v3) are not yet supported; returns NotImplemented. + static Status ValidateAddedDVs(const TableMetadata& metadata, + int64_t starting_snapshot_id, + std::shared_ptr conflict_filter, + const std::shared_ptr& parent, + std::shared_ptr io); + + private: + /// \brief Create a ManifestWriterFactory that records every path it creates in + /// all_written_manifests_. + ManifestWriterFactory MakeTrackedWriterFactory(); + + /// \brief Copy a manifest with the current snapshot ID, for use when snapshot + /// ID inheritance is not possible. + Result CopyManifest(const ManifestFile& manifest); + + /// \brief Write new data manifests for staged data files; caches the result. + Result> WriteNewDataManifests(); + + /// \brief Write new delete manifests for staged delete files; caches the result. + Result> WriteNewDeleteManifests(); + + std::string table_name_; + std::shared_ptr delete_expression_; + bool case_sensitive_ = true; + + // Stable sub-builders for added files — accumulated across retries and merged + // into summary_builder_ at the start of each Apply() call. + SnapshotSummaryBuilder added_data_files_summary_; + SnapshotSummaryBuilder added_delete_files_summary_; + SnapshotSummaryBuilder appended_manifests_summary_; + + ManifestFilterManager data_filter_manager_; + ManifestFilterManager delete_filter_manager_; + ManifestMergeManager data_merge_manager_; + ManifestMergeManager delete_merge_manager_; + + std::unordered_map new_data_files_by_spec_; + std::vector> new_delete_files_; + std::optional new_data_files_data_seq_number_; + + // Manifests passed via AddManifest(): inherit path (no copy needed) and + // rewrite path (must be copied with the current snapshot ID). + std::vector append_manifests_; + std::vector rewritten_append_manifests_; + + // Set to true when new files are staged after the cache was populated, so the + // cache is invalidated and re-written on the next Apply() call (commit retry). + bool has_new_data_files_ = false; + bool has_new_delete_files_ = false; + + std::optional> cached_new_data_manifests_; + std::optional> cached_new_delete_manifests_; + + /// Tracks every manifest path created via MakeTrackedWriterFactory, plus the + /// paths in cached_new_*_manifests_. Used by CleanUncommitted(). + std::unordered_set all_written_manifests_; +}; + +} // namespace iceberg diff --git a/src/iceberg/update/snapshot_update.cc b/src/iceberg/update/snapshot_update.cc index a59ebdc72..849c01c26 100644 --- a/src/iceberg/update/snapshot_update.cc +++ b/src/iceberg/update/snapshot_update.cc @@ -398,14 +398,10 @@ void SnapshotUpdate::CleanAll() { } Status SnapshotUpdate::DeleteFile(const std::string& path) { - static const auto kDefaultDeleteFunc = [this](const std::string& path) { - return this->ctx_->table->io()->DeleteFile(path); - }; if (delete_func_) { return delete_func_(path); - } else { - return kDefaultDeleteFunc(path); } + return ctx_->table->io()->DeleteFile(path); } std::string SnapshotUpdate::ManifestListPath() {