You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2023/08/01 22:25:29 UTC
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11220: ExpressionFilterOperator NULL support.
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org