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


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java:
##########
@@ -59,13 +62,27 @@ protected BlockDocIdSet getTrues() {
   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());
+      BlockDocIdSet trues = filterOperator.getTrues();
+      if (trues instanceof EmptyDocIdSet) {
+        return new MatchAllDocIdSet(_numDocs);
       }
+      if (trues instanceof MatchAllDocIdSet) {
+        continue;
+      }
+      BlockDocIdSet nulls = filterOperator.getNulls();
+      if (_nullHandlingEnabled && !(nulls instanceof EmptyDocIdSet)) {

Review Comment:
   Let's check `_nullHandlingEnabled` before getting `getNulls()`



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java:
##########
@@ -102,11 +103,18 @@ protected BlockDocIdSet getNulls() {
    * @return document IDs in which the predicate evaluates to false.
    */
   protected BlockDocIdSet getFalses() {
-    if (_nullHandlingEnabled) {
-      return new NotDocIdSet(new OrDocIdSet(Arrays.asList(getTrues(), 
getNulls()), _numDocs),
+    BlockDocIdSet trues = getTrues();
+    if (trues instanceof MatchAllDocIdSet) {
+      return EmptyDocIdSet.getInstance();
+    }
+    BlockDocIdSet nulls = getNulls();
+    if (_nullHandlingEnabled && !(nulls instanceof EmptyDocIdSet)) {

Review Comment:
   Same here



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/OrFilterOperator.java:
##########
@@ -59,13 +61,27 @@ protected BlockDocIdSet getTrues() {
   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());
+      BlockDocIdSet trues = filterOperator.getTrues();
+      if (trues instanceof MatchAllDocIdSet) {
+        return EmptyDocIdSet.getInstance();
       }
+      if (trues instanceof EmptyDocIdSet) {
+        continue;
+      }
+      BlockDocIdSet nulls = filterOperator.getNulls();
+      if (_nullHandlingEnabled && !(nulls instanceof EmptyDocIdSet)) {

Review Comment:
   Same here



-- 
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