gortiz commented on code in PR #19408:
URL: https://github.com/apache/pinot/pull/19408#discussion_r4123729071


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/AndDocIdSet.java:
##########
@@ -179,11 +219,64 @@ public BlockDocIdIterator iterator() {
       }
     } else {
       // Otherwise, construct and return an AndDocIdIterator with all 
BlockDocIdIterators.
-
+      // There is no index-based document id set to seed the push-down with, 
so evaluate the deferred composite
+      // children now and let AndDocIdIterator drive them lazily, as it did 
before the push-down existed.
+      for (int i = 0; i < numDocIdSets; i++) {
+        if (allDocIdIterators[i] == null) {
+          allDocIdIterators[i] = docIdSets.get(i).iterator();
+        }
+      }
       return new AndDocIdIterator(allDocIdIterators);
     }
   }
 
+  @Override
+  public boolean isScanBased() {
+    List<BlockDocIdSet> docIdSets = _docIdSets;
+    if (docIdSets == null) {
+      return false;
+    }
+    for (BlockDocIdSet docIdSet : docIdSets) {
+      if (docIdSet.isScanBased()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  @Override
+  public boolean isApplyAndDeferrable() {
+    return isScanBased();
+  }
+
+  /// Intersects this AND with the candidate set by handing the candidates to 
[#iterator] as one more index-based
+  /// child. Everything [#iterator] does -- merging the index children first, 
sorting bitmaps by cardinality,
+  /// 
[org.apache.pinot.common.utils.config.QueryOptionsUtils#isAndScanReorderingEnabled],
 running every scan against
+  /// the merged document ids -- then applies to the restricted evaluation 
too, with one implementation of AND.
+  @Override
+  public ImmutableRoaringBitmap applyAnd(ImmutableRoaringBitmap docIds) {
+    List<BlockDocIdSet> docIdSets = _docIdSets;
+    Preconditions.checkState(docIdSets != null, "applyAnd() called on an 
already consumed AndDocIdSet");
+    if (docIds.isEmpty()) {
+      // No child is evaluated, so none of them will close its own iterator
+      for (BlockDocIdSet docIdSet : docIdSets) {
+        docIdSet.release();
+      }
+      _scanBasedDocIdSets.set(docIdSets);
+      _docIdSets = null;
+      return new MutableRoaringBitmap();
+    }
+    List<BlockDocIdSet> docIdSetsWithCandidates = new 
ArrayList<>(docIdSets.size() + 1);
+    docIdSetsWithCandidates.add(new RangelessBitmapDocIdSet(docIds));
+    docIdSetsWithCandidates.addAll(docIdSets);
+    _docIdSets = docIdSetsWithCandidates;

Review Comment:
   Done in 0c87bb5ae7: `iterator()` keeps the consumed check and delegates to a 
private `buildIterator(List)`, and `applyAnd()` passes 
`docIdSetsWithCandidates` directly. The checks, the stats publication and the 
`_docIdSets = null` step stay in the helper.



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