Jackie-Jiang commented on code in PR #11220:
URL: https://github.com/apache/pinot/pull/11220#discussion_r1281201506
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/ExpressionScanDocIdIterator.java:
##########
@@ -85,7 +88,7 @@ public int next() {
ProjectionBlock projectionBlock =
new ProjectionOperator(_dataSourceMap, new
RangeDocIdSetOperator(blockStartDocId, _blockEndDocId))
.nextBlock();
- RoaringBitmap matchingDocIds = new RoaringBitmap();
+ MutableRoaringBitmap matchingDocIds = new MutableRoaringBitmap();
Review Comment:
`RoaringBitmap` performs better than `MutableRoaringBitmap`. We should
change all of them to use `RoaringBitmap`
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/ExpressionScanDocIdIterator.java:
##########
@@ -212,8 +215,27 @@ private void processProjectionBlock(ProjectionBlock
projectionBlock, BitmapDataP
default:
throw new IllegalStateException();
}
+ if (_nullHandlingEnabled) {
+ RoaringBitmap nullBitmap =
_transformFunction.getNullBitmap(projectionBlock);
+ if (_predicateEvaluationResult == PredicateEvaluationResult.TRUE) {
+ if (nullBitmap != null) {
Review Comment:
Also check if it is empty?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/ExpressionScanDocIdIterator.java:
##########
@@ -212,8 +215,27 @@ private void processProjectionBlock(ProjectionBlock
projectionBlock, BitmapDataP
default:
throw new IllegalStateException();
}
+ if (_nullHandlingEnabled) {
+ RoaringBitmap nullBitmap =
_transformFunction.getNullBitmap(projectionBlock);
Review Comment:
(MAJOR) What is the offset of this bitmap? Is this the docId within the
projection block?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/ExpressionScanDocIdIterator.java:
##########
@@ -212,8 +215,27 @@ private void processProjectionBlock(ProjectionBlock
projectionBlock, BitmapDataP
default:
throw new IllegalStateException();
}
+ if (_nullHandlingEnabled) {
+ RoaringBitmap nullBitmap =
_transformFunction.getNullBitmap(projectionBlock);
+ if (_predicateEvaluationResult == PredicateEvaluationResult.TRUE) {
+ if (nullBitmap != null) {
+ RoaringBitmap fullBitmap = new RoaringBitmap();
+ fullBitmap.add(0L, (long) numDocs);
+ nullBitmap.xor(fullBitmap);
+ matchingDocIds.and(nullBitmap.toMutableRoaringBitmap());
Review Comment:
It is the same as `andNot()`
--
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]
