xiangfu0 commented on code in PR #19297:
URL: https://github.com/apache/pinot/pull/19297#discussion_r3849650322
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ExactVectorScanFilterOperator.java:
##########
@@ -169,87 +184,113 @@ protected void explainAttributes(ExplainAttributeBuilder
attributeBuilder) {
attributeBuilder.putString("vectorLiteral",
Arrays.toString(_predicate.getValue()));
attributeBuilder.putString("fallbackReason",
_vectorExplainContext.getFallbackReason());
attributeBuilder.putLongIdempotent("topKtoSearch", _predicate.getTopK());
+ attributeBuilder.putBool("upsertCandidateFilterApplied",
_requiredUpsertCandidateBitmap != null);
+ attributeBuilder.putLongIdempotent("upsertCandidateFilterCardinality",
getRequiredUpsertCandidateCardinality());
+ attributeBuilder.putLongIdempotent("effectiveAllowedDocIdsCardinality",
+ getEffectiveAllowedDocIdsCardinality());
Review Comment:
Switched both cardinality attributes to additive `putLong` (matching
`annCandidateCount`/`rerankedCandidateCount`), so differing per-segment
valid-doc counts no longer block `PlanNodeMerger`. They are still only emitted
when a candidate scope is present. Applied the same fix to
`VectorSimilarityFilterOperator` in the 4/5 PR, which adds the equivalent
attributes.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ExactVectorScanFilterOperator.java:
##########
@@ -169,87 +184,113 @@ protected void explainAttributes(ExplainAttributeBuilder
attributeBuilder) {
attributeBuilder.putString("vectorLiteral",
Arrays.toString(_predicate.getValue()));
attributeBuilder.putString("fallbackReason",
_vectorExplainContext.getFallbackReason());
attributeBuilder.putLongIdempotent("topKtoSearch", _predicate.getTopK());
+ attributeBuilder.putBool("upsertCandidateFilterApplied",
_requiredUpsertCandidateBitmap != null);
+ attributeBuilder.putLongIdempotent("upsertCandidateFilterCardinality",
getRequiredUpsertCandidateCardinality());
+ attributeBuilder.putLongIdempotent("effectiveAllowedDocIdsCardinality",
+ getEffectiveAllowedDocIdsCardinality());
}
/// Performs brute-force exact search over all documents in the segment.
/// When a distance threshold is set, returns all vectors within the
threshold.
/// Otherwise uses a max-heap to maintain the top-K closest vectors.
- @SuppressWarnings("unchecked")
private ImmutableRoaringBitmap computeExactTopK() {
+ ImmutableRoaringBitmap allowedDocIds = _requiredUpsertCandidateBitmap;
+ if (allowedDocIds != null && allowedDocIds.isEmpty()) {
+ return new MutableRoaringBitmap();
+ }
LOGGER.warn("Performing exact vector scan fallback on column: {} for
segment with {} docs. "
- + "reason={}, distanceFunction={}, hasThreshold={}. "
+ + "reason={}, distanceFunction={}, hasThreshold={},
allowedDocs={}. "
+ "This is expensive -- consider adding a vector index.",
_column, _numDocs, _vectorExplainContext.getFallbackReason(),
- _vectorExplainContext.getDistanceFunction(), _hasDistanceThreshold);
+ _vectorExplainContext.getDistanceFunction(), _hasDistanceThreshold,
+ allowedDocIds != null ? allowedDocIds.getCardinality() : _numDocs);
float[] queryVector = _predicate.getValue();
+ Float threshold = _hasDistanceThreshold ? _distanceThreshold : null;
+ ImmutableRoaringBitmap result = computeExactMatches(_forwardIndexReader,
queryVector, _predicate.getTopK(),
+ _numDocs, _vectorExplainContext.getDistanceFunction(), threshold,
allowedDocIds, _column);
- if (_hasDistanceThreshold) {
- return computeExactThreshold(queryVector);
- }
+ LOGGER.debug("Exact vector scan on column: {} returned {} results from {}
docs",
+ _column, result.getCardinality(), _numDocs);
- int topK = _predicate.getTopK();
+ return result;
+ }
- // Max-heap: entry with largest distance is at the top so we can
efficiently evict it
- PriorityQueue<DocDistance> maxHeap = new PriorityQueue<>(topK + 1,
- (a, b) -> Float.compare(b._distance, a._distance));
+ /// Performs an exact top-K or threshold search over all documents or the
supplied allowed-document bitmap.
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ static ImmutableRoaringBitmap computeExactMatches(ForwardIndexReader<?>
forwardIndexReader,
+ float[] queryVector, int topK, int numDocs,
+ VectorIndexConfig.VectorDistanceFunction distanceFunction, @Nullable
Float distanceThreshold,
+ @Nullable ImmutableRoaringBitmap allowedDocIds, String column) {
+ if ((allowedDocIds != null && allowedDocIds.isEmpty()) ||
(distanceThreshold == null && topK <= 0)) {
+ return new MutableRoaringBitmap();
Review Comment:
Good catch — `topK` comes straight from the query literal with no parse-time
validation, so this was reachable and inconsistent. Top-K mode now does
`Preconditions.checkArgument(topK > 0, ...)`, matching
`IvfFlatVectorIndexReader`/`IvfPqVectorIndexReader`/`IvfOnDiskVectorIndexReader`;
threshold mode still ignores `topK`. Updated the test to expect the error and
added one covering threshold search with a non-positive `topK`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
