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


##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockDocIdSet.java:
##########
@@ -48,12 +49,28 @@ public interface BlockDocIdSet {
    */
   long getNumEntriesScannedInFilter();
 
+  /**
+   * Whether this doc-id set is guaranteed to not match any documents. This 
can be used for short-circuiting operations.
+   */
+  default boolean isAlwaysFalse() {
+    return false;
+  }
+
+  /**
+   * Whether this doc-id set is guaranteed to match all documents. This can be 
used for short-circuiting operations.
+   */
+  default boolean isAlwaysTrue() {
+    return false;
+  }
+
   /**
    * For scan-based FilterBlockDocIdSet, pre-scans the documents and returns a 
non-scan-based FilterBlockDocIdSet.
    */
   default BlockDocIdSet toNonScanDocIdSet() {
     BlockDocIdIterator docIdIterator = iterator();
-
+    if (docIdIterator instanceof EmptyDocIdIterator) {

Review Comment:
   Do we need this change? This is prioritizing the always empty filter case, 
which should be extremely rare



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java:
##########
@@ -53,7 +53,12 @@ protected BlockDocIdSet getTrues() {
     Tracing.activeRecording().setNumChildren(_filterOperators.size());
     List<BlockDocIdSet> blockDocIdSets = new 
ArrayList<>(_filterOperators.size());
     for (BaseFilterOperator filterOperator : _filterOperators) {
-      blockDocIdSets.add(filterOperator.getTrues());
+      BlockDocIdSet blockDocIdSet = filterOperator.getTrues();
+      blockDocIdSets.add(blockDocIdSet);
+      if (blockDocIdSet.isAlwaysFalse()) {
+        // Return AndDocIdSet to ensure that getNumEntriesScannedInFilter is 
correctly reported.
+        return new AndDocIdSet(blockDocIdSets, _queryOptions, true);

Review Comment:
   Do we need to create `AndDocIdSet` here? The scan shouldn't happen yet. The 
overhead of using `AndDocIdSet` is quite high



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to