Copilot commented on code in PR #16755:
URL: https://github.com/apache/pinot/pull/16755#discussion_r2322607888


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SortedDocIdIterator.java:
##########
@@ -28,57 +28,114 @@
  * The {@code SortedDocIdIterator} is the iterator for SortedDocIdSet to 
iterate over a list of matching document id
  * ranges from a sorted column.
  */
-public final class SortedDocIdIterator implements BlockDocIdIterator {
-  private final List<IntPair> _docIdRanges;
-  private final int _numRanges;
+public abstract class SortedDocIdIterator implements BlockDocIdIterator {
+  protected final List<IntPair> _docIdRanges;
+  protected final int _numRanges;
 
-  private int _currentRangeId = 0;
-  private int _nextDocId;
+  protected int _currentRangeId;
+  protected int _nextDocId;
 
-  public SortedDocIdIterator(List<IntPair> docIdRanges) {
+  private SortedDocIdIterator(List<IntPair> docIdRanges) {
     _docIdRanges = docIdRanges;
     _numRanges = _docIdRanges.size();
     _nextDocId = docIdRanges.get(0).getLeft();
   }
 
+  public static SortedDocIdIterator create(List<IntPair> docIdRanges, boolean 
ascending) {
+    return ascending ? new Asc(docIdRanges) : new Desc(docIdRanges);
+  }
+
   public List<IntPair> getDocIdRanges() {
     return _docIdRanges;
   }
 
-  @Override
-  public int next() {
-    IntPair currentRange = _docIdRanges.get(_currentRangeId);
-    if (_nextDocId <= currentRange.getRight()) {
-      // Next document id is within the current range
-      return _nextDocId++;
-    }
-    if (_currentRangeId < _numRanges - 1) {
-      // Move to the next range
-      _currentRangeId++;
-      _nextDocId = _docIdRanges.get(_currentRangeId).getLeft();
-      return _nextDocId++;
-    } else {
-      return Constants.EOF;
+  private static class Asc extends SortedDocIdIterator {
+    private Asc(List<IntPair> docIdRanges) {
+      super(docIdRanges);
+      _currentRangeId = 0;
+      _nextDocId = 0;

Review Comment:
   Line 56 sets `_nextDocId = 0` but it should be set to 
`docIdRanges.get(0).getLeft()` to properly initialize to the start of the first 
range, consistent with the parent constructor logic.
   ```suggestion
         _nextDocId = docIdRanges.get(0).getLeft();
   ```



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