You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Jackie-Jiang (via GitHub)" <gi...@apache.org> on 2026/07/22 01:34:47 UTC
[PR] Implement canProduceBitmaps and getBitmaps for AND and OR filter operators [pinot]
Jackie-Jiang opened a new pull request, #19038:
URL: https://github.com/apache/pinot/pull/19038
## Summary
Implements `canProduceBitmaps` and `getBitmaps` for `AndFilterOperator` and `OrFilterOperator`, which previously fell back to the base `BaseFilterOperator` defaults (`canProduceBitmaps()` returning `false` and `getBitmaps()` throwing).
- `canProduceBitmaps()` returns `true` only when every child can produce bitmaps.
- `getBitmaps()` reduces each child's `BitmapCollection` — so any per-child inversion (exclusive predicates, `NOT`) is materialized before aggregation — then combines them via `BufferFastAggregation.and` (AND) / `BufferFastAggregation.or` (OR), returning a non-inverted single-bitmap `BitmapCollection`.
- `canOptimizeCount()` now delegates to `canProduceBitmaps()` in both operators, removing the duplicated "all children can produce bitmaps" loop it previously carried.
This lets composite AND/OR nodes participate directly in the bitmap-based fast paths (e.g. count optimization and `getFilteredDocIds`) instead of being materialized through document iteration, and it composes recursively for nested AND/OR trees.
`FilterOperatorUtils` strips `MatchAllFilterOperator` / `EmptyFilterOperator` children before constructing an AND/OR, so every child of an AND/OR shares the same `numDocs`, making the reduce-then-aggregate path safe.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Implement canProduceBitmaps and getBitmaps for AND and OR filter operators [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19038:
URL: https://github.com/apache/pinot/pull/19038#discussion_r3626820777
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -106,7 +109,7 @@ public void testComplex() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
This loop only checks that produced docIds match the expected sequence, but it doesn’t assert the OR iterator produced all expected docIds. Add an assertion after the loop to ensure the expected iterator is exhausted.
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -52,7 +55,7 @@ public void testUnionForTwoLists() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
The union assertions only compare elements while the OR iterator produces docIds; if the operator returns a correct prefix but stops early, this test will still pass. Add an assertion after the loop to ensure the expected iterator is fully consumed.
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -77,7 +80,7 @@ public void testUnionForThreeLists() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
The union assertions only validate values emitted by the OR iterator; if it stops early (missing trailing docIds), the test can still pass. Add an assertion after the loop to verify the expected iterator has no remaining elements.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Implement canProduceBitmaps and getBitmaps for AND and OR filter operators [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #19038:
URL: https://github.com/apache/pinot/pull/19038#discussion_r3628024510
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -52,7 +55,7 @@ public void testUnionForTwoLists() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
Good catch. Added `assertFalse(expectedIterator.hasNext());` after the loop so a truncated result set (missing trailing docIds) now fails the test.
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -77,7 +80,7 @@ public void testUnionForThreeLists() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
Good catch. Added `assertFalse(expectedIterator.hasNext());` after the loop so a truncated result set (missing trailing docIds) now fails the test.
##########
pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java:
##########
@@ -106,7 +109,7 @@ public void testComplex() {
BlockDocIdIterator iterator = orOperator.nextBlock().getBlockDocIdSet().iterator();
int docId;
while ((docId = iterator.next()) != Constants.EOF) {
- Assert.assertEquals(docId, expectedIterator.next().intValue());
+ assertEquals(docId, expectedIterator.next().intValue());
}
Review Comment:
Good catch. Added `assertFalse(expectedIterator.hasNext());` after the loop so a truncated result set (missing trailing docIds) now fails the test.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Implement canProduceBitmaps and getBitmaps for AND and OR filter operators [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19038:
URL: https://github.com/apache/pinot/pull/19038#issuecomment-5041121280
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19038?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `20 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 37.73%. Comparing base ([`385e38a`](https://app.codecov.io/gh/apache/pinot/commit/385e38a9c0fe7d09b9e6c549935c61e8e4c92c0c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ab1cbb1`](https://app.codecov.io/gh/apache/pinot/commit/ab1cbb100f73e6030c51ba7a39ef9994bcaf215c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19038?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../pinot/core/operator/filter/AndFilterOperator.java](https://app.codecov.io/gh/apache/pinot/pull/19038?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ffilter%2FAndFilterOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQW5kRmlsdGVyT3BlcmF0b3IuamF2YQ==) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19038?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/pinot/core/operator/filter/OrFilterOperator.java](https://app.codecov.io/gh/apache/pinot/pull/19038?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ffilter%2FOrFilterOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvT3JGaWx0ZXJPcGVyYXRvci5qYXZh) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19038?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (385e38a) and HEAD (ab1cbb1). Click for more details.
>
> <details><summary>HEAD has 16 uploads less than BASE</summary>
>
>| Flag | BASE (385e38a) | HEAD (ab1cbb1) |
>|------|------|------|
>|unittests1|1|0|
>|unittests|2|1|
>|java-25|5|1|
>|temurin|5|1|
>|integration|3|0|
>|integration1|1|0|
>|custom-integration1|1|0|
>|integration2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19038 +/- ##
=============================================
- Coverage 65.45% 37.73% -27.73%
+ Complexity 1421 1414 -7
=============================================
Files 3428 3428
Lines 216598 216608 +10
Branches 34311 34313 +2
=============================================
- Hits 141778 81729 -60049
- Misses 63405 127409 +64004
+ Partials 11415 7470 -3945
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.73% <0.00%> (-27.73%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.73% <0.00%> (-27.73%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.73% <0.00%> (-27.73%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19038/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.73% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19038?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Implement canProduceBitmaps and getBitmaps for AND and OR filter operators [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #19038:
URL: https://github.com/apache/pinot/pull/19038
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org