siddharthteotia commented on a change in pull request #5444: URL: https://github.com/apache/incubator-pinot/pull/5444#discussion_r432322094
########## File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java ########## @@ -25,62 +25,73 @@ import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator; import org.apache.pinot.core.segment.index.readers.InvertedIndexReader; import org.roaringbitmap.buffer.ImmutableRoaringBitmap; +import org.roaringbitmap.buffer.MutableRoaringBitmap; +@SuppressWarnings("rawtypes") public class BitmapBasedFilterOperator extends BaseFilterOperator { private static final String OPERATOR_NAME = "BitmapBasedFilterOperator"; private final PredicateEvaluator _predicateEvaluator; - private final DataSource _dataSource; - private final ImmutableRoaringBitmap[] _bitmaps; - private final int _startDocId; - // TODO: change it to exclusive - // Inclusive - private final int _endDocId; + private final InvertedIndexReader _invertedIndexReader; + private final ImmutableRoaringBitmap _docIds; private final boolean _exclusive; + private final int _numDocs; - BitmapBasedFilterOperator(PredicateEvaluator predicateEvaluator, DataSource dataSource, int startDocId, - int endDocId) { - // NOTE: - // Predicate that is always evaluated as true or false should not be passed into the BitmapBasedFilterOperator for - // performance concern. - // If predicate is always evaluated as true, use MatchAllFilterOperator; if predicate is always evaluated as false, - // use EmptyFilterOperator. - Preconditions.checkArgument(!predicateEvaluator.isAlwaysTrue() && !predicateEvaluator.isAlwaysFalse()); - + BitmapBasedFilterOperator(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs) { _predicateEvaluator = predicateEvaluator; - _dataSource = dataSource; - _bitmaps = null; - _startDocId = startDocId; - _endDocId = endDocId; + _invertedIndexReader = dataSource.getInvertedIndex(); + _docIds = null; _exclusive = predicateEvaluator.isExclusive(); + _numDocs = numDocs; } - public BitmapBasedFilterOperator(ImmutableRoaringBitmap[] bitmaps, int startDocId, int endDocId, boolean exclusive) { + public BitmapBasedFilterOperator(ImmutableRoaringBitmap docIds, boolean exclusive, int numDocs) { _predicateEvaluator = null; - _dataSource = null; - _bitmaps = bitmaps; - _startDocId = startDocId; - _endDocId = endDocId; + _invertedIndexReader = null; + _docIds = docIds; _exclusive = exclusive; + _numDocs = numDocs; } @Override protected FilterBlock getNextBlock() { - if (_bitmaps != null) { - return new FilterBlock(new BitmapDocIdSet(_bitmaps, _startDocId, _endDocId, _exclusive)); + if (_docIds != null) { + if (_exclusive) { + return new FilterBlock(new BitmapDocIdSet(ImmutableRoaringBitmap.flip(_docIds, 0L, _numDocs), _numDocs)); + } else { + return new FilterBlock(new BitmapDocIdSet(_docIds, _numDocs)); + } } int[] dictIds = _exclusive ? _predicateEvaluator.getNonMatchingDictIds() : _predicateEvaluator.getMatchingDictIds(); Review comment: Is is possible to handle NOT_IN, NEQ exactly once? We checked for _exclusive and accordingly get non matching dictIds or matching dictIds based on whether it is true or false. So the predicate is already evaluated correctly. Now why can't we can just work on the docIds for these dictIds. ---------------------------------------------------------------- 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