richardstartin commented on a change in pull request #7435:
URL: https://github.com/apache/pinot/pull/7435#discussion_r709631911



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/RangeIndexReaderImpl.java
##########
@@ -217,4 +272,26 @@ public void close() {
     // NOTE: DO NOT close the PinotDataBuffer here because it is tracked by 
the caller and might be reused later. The
     // caller is responsible of closing the PinotDataBuffer.
   }
+
+  private ImmutableRoaringBitmap getMatchesInRange(int firstRangeId, int 
lastRangeId) {
+    if (firstRangeId == lastRangeId) {
+      return null;
+    }
+    MutableRoaringBitmap matching = new MutableRoaringBitmap();
+    for (int rangeId = firstRangeId + 1; rangeId < lastRangeId; ++rangeId) {

Review comment:
       Actually, Looking back at `findRangeId`, it is ambiguous. -1 can mean 
smaller than the lowest range or greater than the highest range. So this was 
always broken.
   
   This code was lifted straight from `RangeIndexBasedFilterOperator`:
   ```java
   // Ranges in the middle of first and last range are fully matched
        for (int rangeId = firstRangeId + 1; rangeId < lastRangeId; rangeId++) {
          docIds.or(rangeIndexReader.getDocIds(rangeId));
        }
   ```
   
   I don't want existing bugs/specification flaws to get in the way of 
redefining this functionality.




-- 
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]

Reply via email to