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