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


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java:
##########
@@ -126,9 +127,30 @@ public FilteredDocIds getFilteredDocIds() {
     return _filteredDocIds;
   }
 
+  /// Returns the true-document set for execution, reusing an exact bitmap 
that was already materialized through
+  /// [#getFilteredDocIds()] when available. Package-private so boolean filter 
operators can avoid evaluating a scan
+  /// child twice without adding a new external API surface.
+  BlockDocIdSet getTruesForExecution() {

Review Comment:
   Suggest separating this change in a separate PR as this is not closely 
related to the change, and this is quite an important abstraction change, and 
requires more discussion.
   
   Seems this one should be treated as the public facing method. The existing 
`getTrues()`/`getNulls()`/`getFalses()` are internal facing. Should we consider 
renaming existing ones instead?
   Within the current change, only some invocations are switched to this new 
one, and I feel it is actually half done



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