siddharthteotia commented on a change in pull request #5444:
URL: https://github.com/apache/incubator-pinot/pull/5444#discussion_r432323335
##########
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:
I think generally hasNext() should be called before next() like it is
done here. But I think our next() implementations (at least in some of the
cases), have hasNext() called internally from next() as well.
So may be we should simply do `(nextDocId = docIdIterator.next()) < _numDocs`
----------------------------------------------------------------
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
