gortiz commented on code in PR #16789:
URL: https://github.com/apache/pinot/pull/16789#discussion_r2369201265
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/BitmapDocIdSetOperator.java:
##########
@@ -32,51 +33,50 @@
* <p>Should call {@link #nextBlock()} multiple times until it returns
<code>null</code> (already exhausts all the
* documents) or already gathered enough documents (for selection queries).
*/
-public class BitmapDocIdSetOperator extends BaseOperator<DocIdSetBlock> {
+public class BitmapDocIdSetOperator extends BaseDocIdSetOperator {
private static final String EXPLAIN_NAME = "DOC_ID_SET_BITMAP";
-
- // TODO: Consider using BatchIterator to fill the document ids. Currently
BatchIterator only reads bits for one
- // container instead of trying to fill up the buffer with bits from
multiple containers. If in the future
- // BatchIterator provides an API to fill up the buffer, switch to
BatchIterator.
- private final IntIterator _intIterator;
+ @Nullable
+ private IntIteratorDocIdSetOperator _docIdIteratorOperator = null;
private final int[] _docIdBuffer;
+ private final ImmutableBitmapDataProvider _docIds;
+ private final DidOrder _didOrder;
- public BitmapDocIdSetOperator(ImmutableBitmapDataProvider bitmap) {
- _intIterator = bitmap.getIntIterator();
- _docIdBuffer = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ public BitmapDocIdSetOperator(ImmutableBitmapDataProvider docIds, int[]
docIdBuffer, DidOrder didOrder) {
+ _docIds = docIds;
+ _docIdBuffer = docIdBuffer;
+ _didOrder = didOrder;
}
- public BitmapDocIdSetOperator(ImmutableBitmapDataProvider bitmap, int
numDocs) {
- _intIterator = bitmap.getIntIterator();
- _docIdBuffer = new int[Math.min(numDocs,
DocIdSetPlanNode.MAX_DOC_PER_CALL)];
+ public static BitmapDocIdSetOperator ascending(ImmutableBitmapDataProvider
docIds) {
+ return ascending(docIds, new int[DocIdSetPlanNode.MAX_DOC_PER_CALL]);
}
- public BitmapDocIdSetOperator(IntIterator intIterator, int[] docIdBuffer) {
- _intIterator = intIterator;
- _docIdBuffer = docIdBuffer;
+ public static BitmapDocIdSetOperator ascending(ImmutableBitmapDataProvider
docIds, int numDocs) {
+ return ascending(docIds, new int[Math.min(numDocs,
DocIdSetPlanNode.MAX_DOC_PER_CALL)]);
}
- public BitmapDocIdSetOperator(ImmutableBitmapDataProvider bitmap, int[]
docIdBuffer) {
- _intIterator = bitmap.getIntIterator();
- _docIdBuffer = docIdBuffer;
+ public static BitmapDocIdSetOperator ascending(ImmutableBitmapDataProvider
docIds, int[] docIdBuffer) {
+ return new BitmapDocIdSetOperator(docIds, docIdBuffer, DidOrder.ASC);
+ }
+
+ public static BitmapDocIdSetOperator descending(ImmutableBitmapDataProvider
docIds, int numDocs) {
+ return descending(docIds, new int[Math.min(numDocs,
DocIdSetPlanNode.MAX_DOC_PER_CALL)]);
+ }
+
+ public static BitmapDocIdSetOperator descending(ImmutableBitmapDataProvider
bitmap, int[] docIdBuffer) {
+ return new BitmapDocIdSetOperator(bitmap, docIdBuffer, DidOrder.ASC);
}
@Override
protected DocIdSetBlock getNextBlock() {
- int bufferSize = _docIdBuffer.length;
- int index = 0;
- while (index < bufferSize && _intIterator.hasNext()) {
- _docIdBuffer[index++] = _intIterator.next();
- }
- if (index > 0) {
- return new DocIdSetBlock(_docIdBuffer, index);
- } else {
- return null;
+ if (_docIdIteratorOperator == null) {
+ IntIterator iterator = _didOrder == DidOrder.ASC ?
_docIds.getIntIterator() : _docIds.getReverseIntIterator();
+ _docIdIteratorOperator = new IntIteratorDocIdSetOperator(iterator,
_docIdBuffer, _didOrder);
Review Comment:
~it may not be an operator if we prefer it not to be an operator. But I
think this solution is cleaner.~ Edit: I've tried that but it has to be an
operator. Otherwise, we cannot use it in `ExpressionScanDocIdIterator`
About the antipattern.. I agree with you. Again, we can change
`IntIteratorDocIdSetOperator` to not extend Operator. But we are already having
an even worse version of this antipattern when `ExpressionScanDocIdIterator`
creates an Operator when its `applyAnd` method is called.
--
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]
