Jackie-Jiang commented on code in PR #13199:
URL: https://github.com/apache/pinot/pull/13199#discussion_r1612351281


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java:
##########
@@ -60,12 +63,22 @@ protected BlockDocIdSet getFalses() {
     List<BlockDocIdSet> blockDocIdSets = new 
ArrayList<>(_filterOperators.size());
     for (BaseFilterOperator filterOperator : _filterOperators) {
       if (filterOperator.isResultEmpty()) {
-        blockDocIdSets.add(new MatchAllDocIdSet(_numDocs));
-      } else {
-        blockDocIdSets.add(filterOperator.getFalses());
+        return new MatchAllDocIdSet(_numDocs);
       }
+      if (filterOperator.isResultMatchingAll()) {
+        continue;
+      }
+      if (_nullHandlingEnabled) {
+        blockDocIdSets.add(
+            new OrDocIdSet(Arrays.asList(filterOperator.getTrues(), 
filterOperator.getNulls()), _numDocs));

Review Comment:
   In a lot of cases, `getNulls()` can return `EmptyDocIdSet`, in which case we 
should fall back to the case without null handling



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java:
##########
@@ -60,12 +63,22 @@ protected BlockDocIdSet getFalses() {
     List<BlockDocIdSet> blockDocIdSets = new 
ArrayList<>(_filterOperators.size());
     for (BaseFilterOperator filterOperator : _filterOperators) {
       if (filterOperator.isResultEmpty()) {
-        blockDocIdSets.add(new MatchAllDocIdSet(_numDocs));
-      } else {
-        blockDocIdSets.add(filterOperator.getFalses());
+        return new MatchAllDocIdSet(_numDocs);
       }
+      if (filterOperator.isResultMatchingAll()) {
+        continue;
+      }
+      if (_nullHandlingEnabled) {
+        blockDocIdSets.add(
+            new OrDocIdSet(Arrays.asList(filterOperator.getTrues(), 
filterOperator.getNulls()), _numDocs));
+        continue;
+      }
+      blockDocIdSets.add(filterOperator.getTrues());
+    }
+    if (blockDocIdSets.isEmpty()) {
+      return EmptyDocIdSet.getInstance();
     }
-    return new OrDocIdSet(blockDocIdSets, _numDocs);
+    return new NotDocIdSet(new AndDocIdSet(blockDocIdSets, _queryOptions), 
_numDocs);

Review Comment:
   When there is single `BlockDocIdSet`, we want to skip creating the 
`AndDocIdSet`



-- 
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-unsubscr...@pinot.apache.org

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

Reply via email to