Skip to content

Commit 70e8a2c

Browse files
lszskyelszskye
committed
fix xinyu comment
1 parent e5a0200 commit 70e8a2c

3 files changed

Lines changed: 26 additions & 70 deletions

File tree

‎src/paimon/common/utils/optimized_roaring_bitmap64.cpp‎

Lines changed: 21 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -60,46 +60,11 @@ Result<T> ReadLittleEndian(ByteArrayInputStream* input_stream) {
6060

6161
} // namespace
6262

63-
class OptimizedRoaringBitmap64::Impl {
64-
public:
65-
std::vector<roaring::Roaring> bitmaps;
66-
};
67-
68-
OptimizedRoaringBitmap64::OptimizedRoaringBitmap64() : impl_(std::make_unique<Impl>()) {}
69-
70-
OptimizedRoaringBitmap64::~OptimizedRoaringBitmap64() = default;
71-
72-
OptimizedRoaringBitmap64::OptimizedRoaringBitmap64(const OptimizedRoaringBitmap64& other)
73-
: impl_(std::make_unique<Impl>(*other.impl_)) {}
74-
75-
OptimizedRoaringBitmap64& OptimizedRoaringBitmap64::operator=(
76-
const OptimizedRoaringBitmap64& other) {
77-
if (this != &other) {
78-
if (impl_ == nullptr) {
79-
impl_ = std::make_unique<Impl>(*other.impl_);
80-
} else {
81-
*impl_ = *other.impl_;
82-
}
83-
}
84-
return *this;
85-
}
86-
87-
OptimizedRoaringBitmap64::OptimizedRoaringBitmap64(OptimizedRoaringBitmap64&& other) noexcept
88-
: impl_(std::move(other.impl_)) {}
89-
90-
OptimizedRoaringBitmap64& OptimizedRoaringBitmap64::operator=(
91-
OptimizedRoaringBitmap64&& other) noexcept {
92-
if (this != &other) {
93-
impl_ = std::move(other.impl_);
94-
}
95-
return *this;
96-
}
97-
9863
OptimizedRoaringBitmap64 OptimizedRoaringBitmap64::FromRoaringBitmap32(
9964
const RoaringBitmap32& bitmap) {
10065
OptimizedRoaringBitmap64 result;
10166
const auto* roaring_bitmap = static_cast<const roaring::Roaring*>(bitmap.roaring_bitmap_);
102-
result.impl_->bitmaps.push_back(*roaring_bitmap);
67+
result.bitmaps_.push_back(*roaring_bitmap);
10368
return result;
10469
}
10570

@@ -108,7 +73,7 @@ Status OptimizedRoaringBitmap64::Add(int64_t position) {
10873
const auto key = static_cast<int32_t>(position >> 32);
10974
const auto position32 = static_cast<uint32_t>(position);
11075
AllocateBitmapsIfNeeded(static_cast<size_t>(key) + 1);
111-
impl_->bitmaps[key].add(position32);
76+
bitmaps_[key].add(position32);
11277
return Status::OK();
11378
}
11479

@@ -121,9 +86,9 @@ Status OptimizedRoaringBitmap64::AddRange(int64_t start, int64_t end) {
12186

12287
OptimizedRoaringBitmap64& OptimizedRoaringBitmap64::operator|=(
12388
const OptimizedRoaringBitmap64& other) {
124-
AllocateBitmapsIfNeeded(other.impl_->bitmaps.size());
125-
for (size_t key = 0; key < other.impl_->bitmaps.size(); ++key) {
126-
impl_->bitmaps[key] |= other.impl_->bitmaps[key];
89+
AllocateBitmapsIfNeeded(other.bitmaps_.size());
90+
for (size_t key = 0; key < other.bitmaps_.size(); ++key) {
91+
bitmaps_[key] |= other.bitmaps_[key];
12792
}
12893
return *this;
12994
}
@@ -132,8 +97,7 @@ Result<bool> OptimizedRoaringBitmap64::Contains(int64_t position) const {
13297
PAIMON_RETURN_NOT_OK(CheckPosition(position));
13398
const auto key = static_cast<int32_t>(position >> 32);
13499
const auto position32 = static_cast<uint32_t>(position);
135-
return static_cast<size_t>(key) < impl_->bitmaps.size() &&
136-
impl_->bitmaps[key].contains(position32);
100+
return static_cast<size_t>(key) < bitmaps_.size() && bitmaps_[key].contains(position32);
137101
}
138102

139103
bool OptimizedRoaringBitmap64::IsEmpty() const {
@@ -142,36 +106,36 @@ bool OptimizedRoaringBitmap64::IsEmpty() const {
142106

143107
int64_t OptimizedRoaringBitmap64::Cardinality() const {
144108
int64_t cardinality = 0;
145-
for (const roaring::Roaring& bitmap : impl_->bitmaps) {
109+
for (const roaring::Roaring& bitmap : bitmaps_) {
146110
cardinality += static_cast<int64_t>(bitmap.cardinality());
147111
}
148112
return cardinality;
149113
}
150114

151115
bool OptimizedRoaringBitmap64::RunLengthEncode() {
152116
bool changed = false;
153-
for (roaring::Roaring& bitmap : impl_->bitmaps) {
117+
for (roaring::Roaring& bitmap : bitmaps_) {
154118
changed |= bitmap.runOptimize();
155119
}
156120
return changed;
157121
}
158122

159123
void OptimizedRoaringBitmap64::ForEach(const std::function<void(int64_t)>& consumer) const {
160-
for (size_t key = 0; key < impl_->bitmaps.size(); ++key) {
161-
for (uint32_t position32 : impl_->bitmaps[key]) {
124+
for (size_t key = 0; key < bitmaps_.size(); ++key) {
125+
for (uint32_t position32 : bitmaps_[key]) {
162126
const uint64_t position = (static_cast<uint64_t>(key) << 32) | position32;
163127
consumer(static_cast<int64_t>(position));
164128
}
165129
}
166130
}
167131

168132
size_t OptimizedRoaringBitmap64::GetAllocatedBitmapCount() const {
169-
return impl_->bitmaps.size();
133+
return bitmaps_.size();
170134
}
171135

172136
size_t OptimizedRoaringBitmap64::GetSizeInBytes() const {
173137
size_t size = kBitmapCountSizeBytes;
174-
for (const roaring::Roaring& bitmap : impl_->bitmaps) {
138+
for (const roaring::Roaring& bitmap : bitmaps_) {
175139
size += kBitmapKeySizeBytes + bitmap.getSizeInBytes();
176140
}
177141
return size;
@@ -183,10 +147,10 @@ PAIMON_UNIQUE_PTR<Bytes> OptimizedRoaringBitmap64::Serialize(MemoryPool* pool) c
183147
}
184148
PAIMON_UNIQUE_PTR<Bytes> bytes = Bytes::AllocateBytes(GetSizeInBytes(), pool);
185149
char* output = bytes->data();
186-
WriteLittleEndian(static_cast<int64_t>(impl_->bitmaps.size()), &output);
187-
for (size_t key = 0; key < impl_->bitmaps.size(); ++key) {
150+
WriteLittleEndian(static_cast<int64_t>(bitmaps_.size()), &output);
151+
for (size_t key = 0; key < bitmaps_.size(); ++key) {
188152
WriteLittleEndian(static_cast<int32_t>(key), &output);
189-
output += impl_->bitmaps[key].write(output);
153+
output += bitmaps_[key].write(output);
190154
}
191155
return bytes;
192156
}
@@ -197,9 +161,10 @@ Status OptimizedRoaringBitmap64::Deserialize(ByteArrayInputStream* input_stream)
197161
}
198162

199163
PAIMON_ASSIGN_OR_RAISE(int64_t bitmap_count, ReadLittleEndian<int64_t>(input_stream));
200-
if (bitmap_count < 0 || bitmap_count > std::numeric_limits<int32_t>::max()) {
164+
if (bitmap_count < 0) {
201165
return Status::Invalid(fmt::format("Invalid bitmap count: {}", bitmap_count));
202166
}
167+
PAIMON_RETURN_NOT_OK(ValidateValueInRange<int32_t>(bitmap_count, "bitmap count"));
203168

204169
std::vector<roaring::Roaring> bitmaps;
205170
bitmaps.reserve(static_cast<size_t>(bitmap_count));
@@ -232,7 +197,7 @@ Status OptimizedRoaringBitmap64::Deserialize(ByteArrayInputStream* input_stream)
232197
last_key = key;
233198
}
234199

235-
impl_->bitmaps = std::move(bitmaps);
200+
bitmaps_ = std::move(bitmaps);
236201
return Status::OK();
237202
}
238203

@@ -248,7 +213,7 @@ bool OptimizedRoaringBitmap64::operator==(const OptimizedRoaringBitmap64& other)
248213
if (this == &other) {
249214
return true;
250215
}
251-
return impl_->bitmaps == other.impl_->bitmaps;
216+
return bitmaps_ == other.bitmaps_;
252217
}
253218

254219
Status OptimizedRoaringBitmap64::CheckPosition(int64_t position) {
@@ -261,8 +226,8 @@ Status OptimizedRoaringBitmap64::CheckPosition(int64_t position) {
261226
}
262227

263228
void OptimizedRoaringBitmap64::AllocateBitmapsIfNeeded(size_t required_length) {
264-
if (impl_->bitmaps.size() < required_length) {
265-
impl_->bitmaps.resize(required_length);
229+
if (bitmaps_.size() < required_length) {
230+
bitmaps_.resize(required_length);
266231
}
267232
}
268233

‎src/paimon/common/utils/optimized_roaring_bitmap64.h‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
#include <functional>
3030
#include <limits>
3131
#include <memory>
32+
#include <vector>
3233

3334
#include "paimon/io/byte_array_input_stream.h"
3435
#include "paimon/memory/bytes.h"
3536
#include "paimon/memory/memory_pool.h"
3637
#include "paimon/result.h"
3738
#include "paimon/status.h"
3839
#include "paimon/utils/roaring_bitmap32.h"
40+
#include "roaring.hh" // NOLINT(build/include_subdir)
3941

4042
namespace paimon {
4143

@@ -50,15 +52,6 @@ class OptimizedRoaringBitmap64 {
5052
(static_cast<int64_t>(std::numeric_limits<int32_t>::max() - 1) << 32) |
5153
static_cast<uint32_t>(std::numeric_limits<int32_t>::min());
5254

53-
OptimizedRoaringBitmap64();
54-
~OptimizedRoaringBitmap64();
55-
56-
OptimizedRoaringBitmap64(const OptimizedRoaringBitmap64& other);
57-
OptimizedRoaringBitmap64& operator=(const OptimizedRoaringBitmap64& other);
58-
59-
OptimizedRoaringBitmap64(OptimizedRoaringBitmap64&& other) noexcept;
60-
OptimizedRoaringBitmap64& operator=(OptimizedRoaringBitmap64&& other) noexcept;
61-
6255
/// Create an optimized 64-bit bitmap containing all positions from a 32-bit bitmap.
6356
static OptimizedRoaringBitmap64 FromRoaringBitmap32(const RoaringBitmap32& bitmap);
6457

@@ -104,12 +97,10 @@ class OptimizedRoaringBitmap64 {
10497
bool operator==(const OptimizedRoaringBitmap64& other) const noexcept;
10598

10699
private:
107-
class Impl;
108-
109100
static Status CheckPosition(int64_t position);
110101
void AllocateBitmapsIfNeeded(size_t required_length);
111102

112-
std::unique_ptr<Impl> impl_;
103+
std::vector<roaring::Roaring> bitmaps_;
113104
};
114105

115106
} // namespace paimon

‎src/paimon/common/utils/optimized_roaring_bitmap64_test.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,14 +483,14 @@ TEST(OptimizedRoaringBitmap64Test, TestDeserializeRejectsInvalidInput) {
483483
};
484484
ASSERT_NOK_WITH_MSG(bitmap.Deserialize(reinterpret_cast<const char*>(negative_count.data()),
485485
negative_count.size()),
486-
"Invalid bitmap count");
486+
"bitmap count");
487487

488488
const std::vector<uint8_t> too_large_count = {
489489
0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00,
490490
};
491491
ASSERT_NOK_WITH_MSG(bitmap.Deserialize(reinterpret_cast<const char*>(too_large_count.data()),
492492
too_large_count.size()),
493-
"Invalid bitmap count");
493+
"bitmap count");
494494

495495
const std::vector<uint8_t> truncated_key = {
496496
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,

0 commit comments

Comments
 (0)