richardstartin commented on code in PR #9453:
URL: https://github.com/apache/pinot/pull/9453#discussion_r978436827
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java:
##########
@@ -122,35 +157,39 @@ public ImmutableRoaringBitmap
getPartiallyMatchingDocIds(double min, double max)
}
private ImmutableRoaringBitmap queryRangeBitmap(long min, long max, long
columnMax) {
- RangeBitmap rangeBitmap = mapRangeBitmap();
- if (Long.compareUnsigned(max, columnMax) < 0) {
- if (Long.compareUnsigned(min, 0) > 0) {
- return rangeBitmap.between(min, max).toMutableRoaringBitmap();
- }
- return rangeBitmap.lte(max).toMutableRoaringBitmap();
- } else {
- if (Long.compareUnsigned(min, 0) > 0) {
- return rangeBitmap.gte(min).toMutableRoaringBitmap();
- }
+ // Compare unsigned
+ boolean lowerUnbounded = min + Long.MIN_VALUE <= Long.MIN_VALUE;
+ boolean upperUnbounded = max + Long.MIN_VALUE >= columnMax +
Long.MIN_VALUE;
+ if (lowerUnbounded && upperUnbounded) {
MutableRoaringBitmap all = new MutableRoaringBitmap();
- all.add(0, _numDocs);
+ all.add(0L, _numDocs);
return all;
}
+ RangeBitmap rangeBitmap = mapRangeBitmap();
+ if (lowerUnbounded) {
+ return rangeBitmap.lte(max).toMutableRoaringBitmap();
+ }
+ if (upperUnbounded) {
+ return rangeBitmap.gte(min).toMutableRoaringBitmap();
+ }
+ return rangeBitmap.between(min, max).toMutableRoaringBitmap();
}
private int queryRangeBitmapCardinality(long min, long max, long columnMax) {
+ // Compare unsigned
+ boolean lowerUnbounded = min + Long.MIN_VALUE <= Long.MIN_VALUE;
+ boolean upperUnbounded = max + Long.MIN_VALUE >= columnMax +
Long.MIN_VALUE;
+ if (lowerUnbounded && upperUnbounded) {
+ return _numDocs;
+ }
RangeBitmap rangeBitmap = mapRangeBitmap();
- if (Long.compareUnsigned(max, columnMax) < 0) {
- if (Long.compareUnsigned(min, 0) > 0) {
- return (int) rangeBitmap.betweenCardinality(min, max);
- }
+ if (lowerUnbounded) {
return (int) rangeBitmap.lteCardinality(max);
- } else {
- if (Long.compareUnsigned(min, 0) > 0) {
- return (int) rangeBitmap.gteCardinality(min);
- }
- return (int) _numDocs;
}
+ if (upperUnbounded) {
+ return (int) rangeBitmap.gteCardinality(min);
+ }
+ return (int) rangeBitmap.betweenCardinality(min, max);
Review Comment:
These are non-functional changes and I don't think they are improvements, I
suggest removing them to remove noise in the PR (which is an important bug-fix)
--
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]
