Jackie-Jiang commented on a change in pull request #5444:
URL: https://github.com/apache/incubator-pinot/pull/5444#discussion_r434064515



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/AndDocIdIterator.java
##########
@@ -18,112 +18,52 @@
  */
 package org.apache.pinot.core.operator.dociditerators;
 
-import java.util.Arrays;
 import org.apache.pinot.core.common.BlockDocIdIterator;
 import org.apache.pinot.core.common.Constants;
 
 
-// TODO: Optimize this
 public final class AndDocIdIterator implements BlockDocIdIterator {
-  public final BlockDocIdIterator[] docIdIterators;
-  public ScanBasedDocIdIterator[] scanBasedDocIdIterators;
-  public final int[] docIdPointers;
-  public boolean reachedEnd = false;
-  public int currentDocId = -1;
-  int currentMax = -1;
-  private boolean hasScanBasedIterators;
+  public final BlockDocIdIterator[] _docIdIterators;
 
-  public AndDocIdIterator(BlockDocIdIterator[] blockDocIdIterators) {
-    int numIndexBasedIterators = 0;
-    int numScanBasedIterators = 0;
-    for (int i = 0; i < blockDocIdIterators.length; i++) {
-      if (blockDocIdIterators[i] instanceof IndexBasedDocIdIterator) {
-        numIndexBasedIterators = numIndexBasedIterators + 1;
-      } else if (blockDocIdIterators[i] instanceof ScanBasedDocIdIterator) {
-        numScanBasedIterators = numScanBasedIterators + 1;
-      }
-    }
-    // if we have at least one index based then do intersection based on index 
based only, and then
-    // check if matching docs apply on scan based iterator
-    if (numIndexBasedIterators > 0 && numScanBasedIterators > 0) {
-      hasScanBasedIterators = true;
-      int nonScanIteratorsSize = blockDocIdIterators.length - 
numScanBasedIterators;
-      this.docIdIterators = new BlockDocIdIterator[nonScanIteratorsSize];
-      this.scanBasedDocIdIterators = new 
ScanBasedDocIdIterator[numScanBasedIterators];
-      int nonScanBasedIndex = 0;
-      int scanBasedIndex = 0;
-      for (int i = 0; i < blockDocIdIterators.length; i++) {
-        if (blockDocIdIterators[i] instanceof ScanBasedDocIdIterator) {
-          this.scanBasedDocIdIterators[scanBasedIndex++] = 
(ScanBasedDocIdIterator) blockDocIdIterators[i];
-        } else {
-          this.docIdIterators[nonScanBasedIndex++] = blockDocIdIterators[i];
-        }
-      }
-    } else {
-      hasScanBasedIterators = false;
-      this.docIdIterators = blockDocIdIterators;
-    }
-    this.docIdPointers = new int[docIdIterators.length];
-    Arrays.fill(docIdPointers, -1);
-  }
+  private int _nextDocId = 0;
 
-  @Override
-  public int advance(int targetDocId) {
-    if (currentDocId == Constants.EOF) {
-      return currentDocId;
-    }
-    if (currentDocId >= targetDocId) {
-      return currentDocId;
-    }
-    // next() method will always increment currentMax by 1.
-    currentMax = targetDocId - 1;
-    return next();
+  public AndDocIdIterator(BlockDocIdIterator[] docIdIterators) {
+    _docIdIterators = docIdIterators;
   }
 
   @Override
   public int next() {
-    if (currentDocId == Constants.EOF) {
-      return currentDocId;
-    }
-    currentMax = currentMax + 1;
-    // always increment the pointer to current max, when this is called first 
time, every one will
-    // be set to start of posting list.
-    for (int i = 0; i < docIdIterators.length; i++) {
-      docIdPointers[i] = docIdIterators[i].advance(currentMax);
-      if (docIdPointers[i] == Constants.EOF) {
-        reachedEnd = true;
-        currentMax = Constants.EOF;
-        break;
-      }
-      if (docIdPointers[i] > currentMax) {
-        currentMax = docIdPointers[i];
-        if (i > 0) {
-          // we need to advance all pointer since we found a new max
-          i = -1;
-        }
+    int maxDocId = _nextDocId;

Review comment:
       I prefer `AndDocIdIterator` because it performs an AND (intersection) 
operation on all the iterators, and also it is the `BlockDocIdIterator` for 
`AndDocIdSet`. Will add some javadoc showing the relation between them.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to