xiangfu0 opened a new pull request, #19464:
URL: https://github.com/apache/pinot/pull/19464
## Problem
`FilterAwareVectorIndexReader` requires a non-null pre-filter bitmap, but
its five implementations enforced that
four different ways:
| Reader | Before |
|---|---|
| `MutableVectorIndex` | `checkNotNull` → NPE with contract message |
| `HnswVectorIndexReader` | **no guard** — NPE from `BaseFilterQuery`'s
iterator |
| `IvfFlatVectorIndexReader` | **no guard** — NPE on
`preFilterBitmap.isEmpty()` |
| `IvfPqVectorIndexReader` | **no guard** — NPE on
`preFilterBitmap.isEmpty()` |
| `IvfOnDiskVectorIndexReader` | `checkArgument` → IAE, different message |
A caller that lost its filter got a different failure purely based on which
backend indexed the column. In three
cases it was an unmessaged NPE raised from inside Lucene's traversal:
```
Cannot invoke "ImmutableRoaringBitmap.getLongCardinality()" because
"this.this$0._docIds" is null
```
The sole production caller (`VectorSimilarityFilterOperator`) null-checks
before dispatching, so this is a
robustness and diagnosability fix rather than a live bug.
## Change
- State the failure mode once on the SPI method, including that an empty
bitmap admits nothing. Comment-only.
- Guard `IvfFlat` and `IvfPq`; align `IvfOnDisk` on the same exception type
and message.
- Short-circuit an empty bitmap in HNSW before building the query — its
filter clause would otherwise walk every
doc in every leaf, translating ids and testing membership, to assemble an
accept set that admits nothing.
- Report `supportsPreFilter()` in HNSW and `IvfOnDisk` debug info instead of
a hardcoded literal.
## Tests
Every reader now has a null-contract test. Two tests are restored from
#19303, which dropped them before merge:
the mutable null-bitmap rejection, and the IVF_PQ assertion that candidate
generation honors the pre-filter — it
asserts up front that the allowed docs sit *outside* the unfiltered top-K,
so an implementation that intersected
afterwards would return empty and fail.
Both new guards were mutation-tested rather than assumed:
- Delete the null guard → `testFilteredReaderRejectsNullBitmap` fails with
the raw `getLongCardinality()` NPE above.
- Delete the empty short-circuit →
`testFilteredReaderShortCircuitsEmptyBitmapBeforeBuildingQuery` fails with
`vectorUseBoundedQueue=false requires vectorEfSearch to be set`, proving
the short-circuit runs before query
construction. (An assertion that the result is merely empty does **not**
discriminate — Lucene returns no hits
for a zero-match filter anyway.)
77 vector tests pass in `pinot-segment-local`; spotless, checkstyle and
license are clean.
Follow-up to #19303.
--
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]
