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



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java
##########
@@ -18,238 +18,138 @@
  */
 package org.apache.pinot.core.operator.dociditerators;
 
-import org.apache.pinot.core.common.BlockMetadata;
 import org.apache.pinot.core.common.BlockSingleValIterator;
-import org.apache.pinot.core.common.BlockValSet;
 import org.apache.pinot.core.common.Constants;
 import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
-import org.apache.pinot.spi.data.FieldSpec;
 import org.roaringbitmap.IntIterator;
 import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
 import org.roaringbitmap.buffer.MutableRoaringBitmap;
 
 
-public class SVScanDocIdIterator implements ScanBasedDocIdIterator {
-  private int _currentDocId = -1;
+public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
+  private final PredicateEvaluator _predicateEvaluator;
   private final BlockSingleValIterator _valueIterator;
-  private int _startDocId;
-  private int _endDocId;
-  private PredicateEvaluator _evaluator;
-  private String _operatorName;
-  private int _numEntriesScanned = 0;
+  private final int _numDocs;
   private final ValueMatcher _valueMatcher;
 
-  public SVScanDocIdIterator(String operatorName, BlockValSet blockValSet, 
BlockMetadata blockMetadata,
-      PredicateEvaluator evaluator) {
-    _operatorName = operatorName;
-    _evaluator = evaluator;
-    _valueIterator = (BlockSingleValIterator) blockValSet.iterator();
-
-    if (evaluator.isAlwaysFalse()) {
-      _currentDocId = Constants.EOF;
-      setStartDocId(Constants.EOF);
-      setEndDocId(Constants.EOF);
-    } else {
-      setStartDocId(blockMetadata.getStartDocId());
-      setEndDocId(blockMetadata.getEndDocId());
-    }
+  private int _nextDocId = 0;
+  private long _numEntriesScanned = 0L;
 
-    if (evaluator.isDictionaryBased()) {
-      _valueMatcher = new IntMatcher(); // Match using dictionary id's that 
are integers.
-    } else {
-      _valueMatcher = getValueMatcherForType(blockMetadata.getDataType());
-    }
-    _valueMatcher.setEvaluator(evaluator);
-  }
-
-  /**
-   * After setting the startDocId, next calls will always return from 
>=startDocId
-   *
-   * @param startDocId Start doc id
-   */
-  public void setStartDocId(int startDocId) {
-    _currentDocId = startDocId - 1;
-    _valueIterator.skipTo(startDocId);
-    _startDocId = startDocId;
-  }
-
-  /**
-   * After setting the endDocId, next call will return Constants.EOF after 
currentDocId exceeds
-   * endDocId
-   *
-   * @param endDocId End doc id
-   */
-  public void setEndDocId(int endDocId) {
-    _endDocId = endDocId;
-  }
-
-  @Override
-  public boolean isMatch(int docId) {
-    if (_currentDocId == Constants.EOF) {
-      return false;
-    }
-    _valueIterator.skipTo(docId);
-    _numEntriesScanned++;
-    return _valueMatcher.doesCurrentEntryMatch(_valueIterator);
-  }
-
-  @Override
-  public int advance(int targetDocId) {
-    if (_currentDocId == Constants.EOF) {
-      return _currentDocId;
-    }
-    if (targetDocId < _startDocId) {
-      targetDocId = _startDocId;
-    } else if (targetDocId > _endDocId) {
-      _currentDocId = Constants.EOF;
-    }
-    if (_currentDocId >= targetDocId) {
-      return _currentDocId;
-    } else {
-      _currentDocId = targetDocId - 1;
-      _valueIterator.skipTo(targetDocId);
-      return next();
-    }
+  public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, 
BlockSingleValIterator valueIterator, int numDocs) {
+    _predicateEvaluator = predicateEvaluator;
+    _valueIterator = valueIterator;
+    _numDocs = numDocs;
+    _valueMatcher = getValueMatcher();
   }
 
   @Override
   public int next() {
-    if (_currentDocId == Constants.EOF) {
-      return Constants.EOF;
-    }
-    while (_valueIterator.hasNext() && _currentDocId < _endDocId) {
-      _currentDocId = _currentDocId + 1;
+    while (_nextDocId < _numDocs) {
+      int nextDocId = _nextDocId++;
       _numEntriesScanned++;
-      if (_valueMatcher.doesCurrentEntryMatch(_valueIterator)) {
-        return _currentDocId;
+      if (_valueMatcher.doesNextValueMatch()) {
+        return nextDocId;
       }
     }
-    _currentDocId = Constants.EOF;
     return Constants.EOF;
   }
 
   @Override
-  public int currentDocId() {
-    return _currentDocId;
-  }
-
-  @Override
-  public String toString() {
-    return SVScanDocIdIterator.class.getSimpleName() + "[" + _operatorName + 
"]";
+  public int advance(int targetDocId) {
+    _nextDocId = targetDocId;
+    _valueIterator.skipTo(targetDocId);
+    return next();
   }
 
   @Override
   public MutableRoaringBitmap applyAnd(ImmutableRoaringBitmap docIds) {
     MutableRoaringBitmap result = new MutableRoaringBitmap();
-    if (_evaluator.isAlwaysFalse()) {
-      return result;
-    }
-    IntIterator intIterator = docIds.getIntIterator();
-    int docId = -1;
-    while (intIterator.hasNext() && docId < _endDocId) {
-      docId = intIterator.next();
-      if (docId >= _startDocId) {
-        _valueIterator.skipTo(docId);
-        _numEntriesScanned++;
-        if (_valueMatcher.doesCurrentEntryMatch(_valueIterator)) {
-          result.add(docId);
-        }
+    IntIterator docIdIterator = docIds.getIntIterator();
+    int nextDocId;
+    while (docIdIterator.hasNext() && (nextDocId = docIdIterator.next()) < 
_numDocs) {

Review comment:
       its ok to drop hasNext if it simplifies the implementation




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