SteNicholas commented on code in PR #472:
URL: https://github.com/apache/paimon-cpp/pull/472#discussion_r4388523524
##########
src/paimon/core/operation/data_evolution_split_read.cpp:
##########
@@ -555,10 +592,38 @@ Result<bool> DataEvolutionSplitRead::SkipByFileIndex(
file,
options_.GetFileSystem(), pool_));
PAIMON_ASSIGN_OR_RAISE(bool is_remain, index_result->IsRemain());
if (!is_remain) {
- return true;
+ return std::optional<std::vector<Range>>(std::vector<Range>());
+ }
+ if (!can_push_down_selection) {
+ continue;
+ }
+ if (auto* bitmap_index =
dynamic_cast<BitmapIndexResult*>(index_result.get())) {
+ PAIMON_ASSIGN_OR_RAISE(const RoaringBitmap32* bitmap,
bitmap_index->GetBitmap());
+ if (selection) {
+ *selection &= *bitmap;
+ } else {
+ selection = *bitmap;
+ PAIMON_ASSIGN_OR_RAISE(first_row_id,
file->NonNullFirstRowId());
+ }
+ }
+ }
+ if (!selection) {
+ return selected_ranges;
+ }
+ *has_bitmap_selection = true;
+ std::vector<Range> index_ranges;
+ for (auto iter = selection->Begin(); iter != selection->End(); ++iter) {
+ int64_t row_id = *first_row_id + static_cast<uint32_t>(*iter);
+ if (!index_ranges.empty() && index_ranges.back().to + 1 == row_id) {
+ index_ranges.back().to = row_id;
+ } else {
+ index_ranges.emplace_back(row_id, row_id);
Review Comment:
**Intersect the existing row selection before expanding bitmap hits**. This
loop expands every file-index hit into `index_ranges` before applying
`selected_ranges` below, so even an indexed split selecting a single row pays
the allocation cost for the entire file's bitmap. In a standalone C++
reproducer of this loop using the vendored Roaring implementation, 10 million
rows with alternating hits produced a 1,254,608-byte serialized bitmap but 5
million `Range` entries (80 MB of payload and 128 MiB of vector capacity), all
to retain one range after intersection. This now also affects single-file
groups, which previously intersected the row-selection and index bitmaps
directly. Please intersect the existing row selection while still in bitmap
form, or restrict iteration to the selected ranges, before materializing
ranges; otherwise large files can incur substantial unnecessary allocations and
OOMs.
--
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]
