xiangfu0 commented on code in PR #19287:
URL: https://github.com/apache/pinot/pull/19287#discussion_r3801908973
##########
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() {
+ FilteredDocIds filteredDocIds = _filteredDocIds;
+ if (filteredDocIds == null) {
+ return getTrues();
+ }
+ ImmutableRoaringBitmap docIds = filteredDocIds.getDocIds();
+ if (docIds == null) {
+ return new MatchAllDocIdSet(_numDocs);
+ }
+ long numEntriesScannedInFilter =
filteredDocIds.getNumEntriesScannedInFilter();
+ return new BitmapDocIdSet(docIds, _numDocs) {
+ @Override
+ public long getNumEntriesScannedInFilter() {
+ return numEntriesScannedInFilter;
+ }
+ };
+ }
Review Comment:
Thanks. I traced every cache assignment: the only null docIds value is
created by the statically match-all branch as FilteredDocIds(null, 0L). Scan
materialization always caches a non-null bitmap together with its scan count,
including when every document matches, and both the cache field and constructor
are private. Therefore the alleged null-plus-nonzero-count state is unreachable
and current execution accounting is preserved; no production change is needed.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/VectorSimilarityFilterOperator.java:
##########
@@ -292,31 +381,80 @@ protected void explainAttributes(ExplainAttributeBuilder
attributeBuilder) {
if (explainContext.getFilterSelectivity() >= 0) {
attributeBuilder.putString("filterSelectivity", String.format("%.4f",
explainContext.getFilterSelectivity()));
}
+ attributeBuilder.putBool("upsertCandidateFilterApplied",
_requiredUpsertCandidateBitmap != null);
+ attributeBuilder.putLongIdempotent("upsertCandidateFilterCardinality",
getRequiredUpsertCandidateCardinality());
+ attributeBuilder.putBool("publishedDocRangeApplied",
_publishedDocIdsBitmap != null);
+ attributeBuilder.putLongIdempotent("publishedDocRangeCardinality",
getPublishedDocIdsCardinality());
+ attributeBuilder.putBool("requiredMetadataCandidateFilterApplied",
_requiredMetadataCandidateBitmap != null);
+
attributeBuilder.putLongIdempotent("requiredMetadataCandidateFilterCardinality",
+ getRequiredMetadataCandidateCardinality());
+ attributeBuilder.putLongIdempotent("effectiveAllowedDocIdsCardinality",
+ getEffectiveAllowedDocIdsCardinality());
+ attributeBuilder.putBool("candidateGenerationSkipped",
_candidateGenerationSkipped);
+ if (_candidateGenerationSkipped) {
+ attributeBuilder.putString("candidateGenerationSkipReason",
getCandidateGenerationSkipReason());
+ }
+ if (_runtimeFallbackReason != null) {
+ attributeBuilder.putString("fallbackReason", _runtimeFallbackReason);
+ }
}
/// Returns true if the underlying vector index reader supports pre-filter
ANN search.
public boolean supportsPreFilter() {
- return _vectorIndexReader instanceof FilterAwareVectorIndexReader
- && ((FilterAwareVectorIndexReader)
_vectorIndexReader).supportsPreFilter();
+ return readerSupportsPreFilter();
}
/// Executes the vector search with backend-specific parameter dispatch and
optional rerank.
private ImmutableRoaringBitmap executeSearch() {
String column = _predicate.getLhs().getIdentifier();
float[] queryVector = _predicate.getValue();
VectorExplainContext explainContext = _vectorExplainContext;
+ boolean backendParamsConfigured = false;
+ boolean searchExecuted = false;
try {
+ ImmutableRoaringBitmap effectiveAllowedDocIds = _effectiveAllowedDocIds;
+ if (hasMandatoryCandidateScope() && effectiveAllowedDocIds.isEmpty()) {
+ _candidateGenerationSkipped = true;
+ _annCandidateCount = 0;
+ _rerankedCandidateCount = 0;
+ return new MutableRoaringBitmap();
+ }
+ _candidateGenerationSkipped = false;
+
+ if (hasMandatoryCandidateScope() && !readerSupportsPreFilter()) {
+ if (_forwardIndexReader == null) {
+ throw new IllegalStateException("Cannot honor mandatory vector
candidate scope on vector column: "
+ + column + " -- vector index reader does not support filtered
search and no forward index is available");
+ }
+ _vectorSearchMode = VectorSearchMode.EXACT_SCAN;
+ _runtimeFallbackReason = _requiredCandidateFallbackReason != null ?
_requiredCandidateFallbackReason
+ : "vector_index_not_filter_aware_for_mandatory_scope";
+
VectorSearchMetrics.getInstance().recordFallback(_runtimeFallbackReason);
+ LOGGER.warn("Performing exact allowed-document vector scan on column:
{} because {}. allowedDocs={}",
+ column, _runtimeFallbackReason,
effectiveAllowedDocIds.getCardinality());
+ Float threshold = _hasThresholdPredicate ? _distanceThreshold : null;
+ searchExecuted = true;
+ ImmutableRoaringBitmap exactResults =
ExactVectorScanFilterOperator.computeExactMatches(
+ _forwardIndexReader, queryVector, _predicate.getTopK(), _numDocs,
_distanceFunction, threshold,
+ effectiveAllowedDocIds, column);
+ _annCandidateCount = -1;
+ _rerankedCandidateCount = exactResults.getCardinality();
+ return exactResults;
+ }
+
// 1. Configure backend-specific parameters via interfaces
configureBackendParams(column);
- refreshExplainContext(null);
+ backendParamsConfigured = true;
+ refreshExplainContext();
explainContext = _vectorExplainContext;
Review Comment:
Fixed in 2c163f07e9. The operator now claims cleanup responsibility before
the first backend setter and nests cleanup in its own finally so a partial
configuration or explain refresh failure cannot skip it. The new regression
configures nprobe, throws while setting efSearch, verifies every cleanup method
runs, and verifies ANN search never starts. The focused core run passed 64
tests.
--
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]
