github-actions[bot] commented on code in PR #65421:
URL: https://github.com/apache/doris/pull/65421#discussion_r3576144588
##########
be/src/core/value/bitmap_value.h:
##########
@@ -1980,51 +2475,34 @@ class BitmapValue {
throw Exception(ErrorCode::INTERNAL_ERROR,
"bitmap value with incorrect set count, count:
{}", count);
}
- _set.reserve(count);
- for (uint8_t i = 0; i != count; ++i, src += sizeof(uint64_t)) {
- _set.insert(decode_fixed64_le(reinterpret_cast<const
uint8_t*>(src)));
- }
- if (_set.size() != count) {
+ if (!_set.read(src, count)) {
throw Exception(ErrorCode::INTERNAL_ERROR,
- "bitmap value with incorrect set count, count:
{}, set size: {}",
- count, _set.size());
+ "bitmap value with duplicated set values,
count: {}", count);
}
+ src += sizeof(uint64_t) * count;
if (!config::enable_set_in_bitmap_value) {
- _prepare_bitmap_for_write();
- for (auto v : _set) {
- _bitmap->add(v);
- }
- _type = BITMAP;
- _set.clear();
+ _convert_set_to_bitmap();
Review Comment:
This path can still reuse an old inactive `_bitmap` when `deserialize()` is
called on an existing `BitmapValue`. `DataTypeBitMap::deserialize()` resizes
the existing `ColumnBitmap` vector and then calls
`data[i].deserialize(data_ptr)`, so an element that previously held a BITMAP
can be overwritten by a SET payload without dropping the old roaring map. If
SET storage is disabled here, or if the freshly deserialized SET is later
promoted, `_convert_set_to_bitmap()` only calls `_prepare_bitmap_for_write()`
and then `addMany(_set...)`, so it appends the new SET values into the stale
bitmap instead of replacing it. For example, a reused value that had BITMAP
`{0..40}` and then deserializes SET `{100,101}` can become `{0..40,100,101}`
after conversion. Please reset/clear inactive representation state at the start
of `deserialize()` or make the deserialize SET-to-BITMAP conversion build from
an empty roaring map, and add a reuse test for BITMAP-to-SET deserialization
followed by SET prom
otion.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]