SteNicholas commented on code in PR #472:
URL: https://github.com/apache/paimon-cpp/pull/472#discussion_r4396736118
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -1388,8 +1469,70 @@ Result<DeletionVector::Factory>
DataEvolutionSplitRead::CreateGroupDvFactory(
});
}
+Result<RoaringBitmap32> DataEvolutionSplitRead::ExcludeDeletedPositions(
+ RoaringBitmap32 selection, const std::shared_ptr<DeletionVector>&
deletion_vector) {
+ if (selection.IsEmpty() || deletion_vector->IsEmpty()) {
+ return selection;
+ }
+ if (auto* bitmap_dv =
dynamic_cast<BitmapDeletionVector*>(deletion_vector.get())) {
+ selection -= *bitmap_dv->GetBitmap();
+ return selection;
+ }
+ PAIMON_ASSIGN_OR_RAISE(int64_t cardinality,
deletion_vector->GetCardinality());
+ if (cardinality < selection.Cardinality()) {
+ RoaringBitmap32 deleted;
+
PAIMON_RETURN_NOT_OK(deletion_vector->ForEachDeletedPosition([&](int64_t
position) {
+ if (position >= 0 && position <=
std::numeric_limits<uint32_t>::max()) {
+ deleted.Add(static_cast<int32_t>(position));
+ }
+ }));
+ selection -= deleted;
+ return selection;
+ }
+ RoaringBitmap32 live;
+ const auto end = selection.End();
+ for (auto iter = selection.Begin(); iter != end; ++iter) {
+ PAIMON_ASSIGN_OR_RAISE(bool is_deleted,
+
deletion_vector->IsDeleted(static_cast<uint32_t>(*iter)));
+ if (!is_deleted) {
+ live.Add(*iter);
+ }
+ }
+ return live;
+}
+
Result<std::vector<Range>> DataEvolutionSplitRead::ExcludeDeletedRowIds(
const std::vector<Range>& ranges, const GroupDeletionVector& group_dv) {
+ if (ranges.empty()) {
+ return std::vector<Range>();
+ }
+ PAIMON_ASSIGN_OR_RAISE(int64_t cardinality,
group_dv.deletion_vector->GetCardinality());
+ int64_t selected_count = 0;
+ for (const auto& range : ranges) {
+ if (range.Count() > cardinality - selected_count) {
+ // Sparse deletions in a large selection: visit deleted positions
instead of every
+ // selected row. Coalesce runs before taking the complement to
keep memory bounded
+ // for contiguous deletions.
+ std::vector<Range> deleted;
+ PAIMON_RETURN_NOT_OK(
+ group_dv.deletion_vector->ForEachDeletedPosition([&](int64_t
position) {
+ if (position < 0 ||
+ position > group_dv.anchor_range.to -
group_dv.anchor_range.from) {
+ return;
+ }
+ int64_t row_id = group_dv.anchor_range.from + position;
+ if (!deleted.empty() && deleted.back().to < row_id &&
+ row_id - deleted.back().to == 1) {
+ deleted.back().to = row_id;
+ } else {
+ deleted.emplace_back(row_id, row_id);
Review Comment:
**Restrict deletion expansion to the selected Blob gap.** When a Blob layer
has a large gap but deletions lie elsewhere in the anchor, this callback still
materializes every deletion as a `Range`; `Range::Exclude` then copies that
vector before the intersection with `ranges`. For example, with anchor `[0,
3199999]`, selected gap `[2000000, 3199999]`, and one million deletions at even
positions in `[0, 1999999]`, the result is one unchanged range, but the new
branch allocates one million unnecessary range entries. A standalone C++
reproducer using this helper's body, the repository's `Range` implementation,
and vendored Roaring measured about 49 MiB peak RSS versus 2 MiB with the
previous algorithm; the serialized deletion bitmap was only 254,208 bytes.
Please filter deleted row IDs against the selected ranges before storing them,
or compute the complement while streaming, to avoid this allocation growth and
potential OOMs on larger files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
