github-actions[bot] commented on code in PR #65421:
URL: https://github.com/apache/doris/pull/65421#discussion_r3584724417
##########
be/src/core/value/bitmap_value.h:
##########
@@ -798,7 +1065,7 @@ class Roaring64MapSetBitForwardIterator {
return orig;
}
- bool move(const uint64_t& x) {
+ bool move_equalorlarger(const value_type& x) {
Review Comment:
`move_equalorlarger()` needs the same empty-inner skip that the constructor
and `operator++()` use before it returns true. Right now it can land on an
empty outer bucket and still look valid. One reachable case is a BITMAP with
one value in high bucket 1 and more than 32 values in high bucket 2: after
removing the high-1 value, `Roaring64Map::remove()` leaves an empty bucket 1
while `BitmapValue` stays BITMAP. A later `contains_any(0, 1ULL << 32)` calls
`lower_bound(0)`, this function returns true on that empty bucket without
checking `i.has_value`, and `operator*()` can synthesize `1 << 32`, so the
range check reports a value that is not actually present. Please advance past
empty inner bitmaps after every `roaring_init_iterator()` path here, and add a
unit case that removes the only value from a lower high bucket while later
buckets keep the bitmap representation.
--
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]
