Tan-JiaLiang commented on code in PR #5028:
URL: https://github.com/apache/paimon/pull/5028#discussion_r1945994850
##########
paimon-common/src/main/java/org/apache/paimon/fileindex/bitmap/BitmapFileIndex.java:
##########
@@ -235,8 +271,18 @@ private RoaringBitmap32 readBitmap(Object bitmapId) {
if (offset < 0) {
return RoaringBitmap32.bitmapOf(-1 - offset);
} else {
- seekableInputStream.seek(bodyStart + offset);
+
seekableInputStream.seek(bitmapFileIndexMeta.getBodyStart() + offset);
Review Comment:
It makes me feels obsessive-compulsive. So I want to suggest one more time.
For V2: find the blocks and then find the entry, only 2 binary search, but
now need to binary search six times.
What do you think about this? Using the `findEntry` instead of
`contains`/`getOffset`/`getLength`.
```java
private RoaringBitmap32 readBitmap(Object bitmapId) {
Entry entry = bitmapFileIndexMeta.findEntry(bitmapId);
if (entry == null) {
return new RoaringBitmap32();
}
int offset = entry.getOffset();
int length = entry.getLength();
if (offset < 0) {
return RoaringBitmap32.bitmapOf(-1 - offset);
}
try {
seekableInputStream.seek(bitmapFileIndexMeta.getBodyStart() +
offset);
RoaringBitmap32 bitmap = new RoaringBitmap32();
if (enableNextOffsetToSize && length != -1) {
DataInputStream input = new DataInputStream(seekableInputStream);
byte[] bytes = new byte[length];
input.readFully(bytes);
bitmap.deserialize(ByteBuffer.wrap(bytes));
} else {
bitmap.deserialize(new DataInputStream(seekableInputStream));
}
return bitmap;
} catch (Exception e) {
throw new RuntimeException(e);
}
}
```
--
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]
