Skip to content

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Dec 30, 2025

No description provided.

@wgtmac wgtmac force-pushed the manifest_group branch 2 times, most recently from 79f27c9 to 1635143 Compare December 30, 2025 05:39
Comment on lines 112 to 117
auto old_predicate = std::move(manifest_entry_predicate_);
manifest_entry_predicate_ = [old_predicate = std::move(old_predicate),
predicate =
std::move(predicate)](const ManifestEntry& entry) {
return old_predicate(entry) && predicate(entry);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto old_predicate = std::move(manifest_entry_predicate_);
manifest_entry_predicate_ = [old_predicate = std::move(old_predicate),
predicate =
std::move(predicate)](const ManifestEntry& entry) {
return old_predicate(entry) && predicate(entry);
};
manifest_entry_predicate_ = [old_predicate = std::move(manifest_entry_predicate_),
predicate =
std::move(predicate)](const ManifestEntry& entry) {
return old_predicate(entry) && predicate(entry);
};

std::vector<std::shared_ptr<FileScanTask>> file_tasks;
file_tasks.reserve(tasks.size());
for (auto& task : tasks) {
file_tasks.push_back(std::static_pointer_cast<FileScanTask>(task));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
file_tasks.push_back(std::static_pointer_cast<FileScanTask>(task));
file_tasks.push_back(internal::checked_pointer_cast<FileScanTask>(task));

ICEBERG_ASSIGN_OR_RAISE(auto manifest_evaluator, get_manifest_evaluator(spec_id));
ICEBERG_ASSIGN_OR_RAISE(bool should_match, manifest_evaluator->Evaluate(manifest));
if (!should_match) {
continue; // Skip this manifest because it doesn't match partition filter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
continue; // Skip this manifest because it doesn't match partition filter
// Skip this manifest because it doesn't match partition filter
continue;

constexpr int64_t kInvalidSnapshotId = -1;
/// \brief Stand-in for the current sequence number that will be assigned when the commit
/// is successful. This is replaced when writing a manifest list by the ManifestFile
/// wrapper
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wrapper -> adapter, which matches the code better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants