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