Enforce the upsert doc-ids snapshot in vector candidate generation - #19303
Open
xiangfu0 wants to merge 1 commit into
Open
Enforce the upsert doc-ids snapshot in vector candidate generation#19303xiangfu0 wants to merge 1 commit into
xiangfu0 wants to merge 1 commit into
Conversation
On FULL-upsert tables, VECTOR_SIMILARITY spent its per-segment top-K candidate budget on all physical rows and the doc-ids snapshot was only ANDed afterwards, so obsolete row versions could consume the K slots and crowd out nearer current rows. The snapshot is now a required candidate filter enforced before top-K selection in every vector path (VECTOR_SIMILARITY and VECTOR_SIMILARITY_RADIUS, filtered ANN and exact-scan fallback), with the outer bitmap AND retained as defense in depth. - FilterPlanNode passes a clamped defensive copy of the snapshot into the vector operators; empty snapshots plan an EmptyFilterOperator; unfiltered ANN is refused (fail loud) whenever a required filter cannot be honored. - MutableVectorIndex implements FilterAwareVectorIndexReader: stores the supplied Pinot doc id, translates hits through it instead of assuming ScoreDoc.doc == docId (fixes a latent bug on merge-renumbered indexes), and serves filtered search from a SearcherManager NRT view so uncommitted rows are visible. - ExactVectorScanFilterOperator and VectorRadiusFilterOperator scan only the required doc ids when present; explain output reports the applied filter cardinality, search mode and fallback reason. - VectorSearchSpec construction context replaces the growing operator constructor ladders; shared BasePinotDocIdBitmapFilterQuery backs both Lucene bitmap filter queries. - VectorIndexType.validate now rejects dictionary-encoded vector columns (they fail at query time on every forward-index read path). Backward incompatible for previously-accepted misconfigured table configs: declare encodingType RAW / noDictionary for vector columns.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19303 +/- ##
=============================================
- Coverage 67.10% 39.19% -27.91%
+ Complexity 1424 1423 -1
=============================================
Files 3459 3464 +5
Lines 219697 220567 +870
Branches 34985 35191 +206
=============================================
- Hits 147432 86459 -60973
- Misses 60488 126228 +65740
+ Partials 11777 7880 -3897
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On FULL-upsert tables,
WHERE vectorSimilarity(col, query, K)planned the ANN search over all physical rows and only ANDed the upsert doc-ids snapshot around the result afterwards (FilterPlanNode.run()). Obsolete row versions that are physically nearest to the query consume the per-segment top-K candidate budget before the snapshot removes them, so a segment can return fewer than K rows and omit nearer current rows entirely.Repro shape: entity A's old version sits at the query vector, its new version far away, entities B/C in between. The old A rows win the K candidate slots, get removed by the snapshot AND, and B/C never become candidates — the query returns the wrong entities. The new integration test's
OPTION(skipUpsert=true)control query demonstrates the obsolete rows are physically nearest.What this PR does
The upsert doc-ids snapshot is now a required candidate filter, enforced before top-K selection on every vector path, and kept strictly separate from the optional metadata prefilter (which retains its adaptive
VectorSearchStrategybehavior). The outer bitmap AND is retained as defense in depth. This lands as three units (bundled because the integration test exercises them together):1. Required-filter enforcement (pinot-core)
FilterPlanNodecaptures a defensive copy of the snapshot whenever the filter tree contains a vector predicate (any depth — AND/OR/NOT), clamps it to the planned doc range (underConsistencyMode.NONEthe snapshot can reference a row still being written), passes it into the vector operators via the newVectorSearchSpecconstruction context, and uses the same instance for the outer AND.VectorSimilarityFilterOperator: with a required filter present, candidate generation always uses the filter-aware 3-arggetDocIds(intersected with the optional metadata prefilter when both apply,effectiveAllowed = required ∩ optional); an empty snapshot/intersection returns empty without invoking the reader; unfiltered ANN is refused (IllegalStateException) rather than silently run.ExactVectorScanFilterOperatorandVectorRadiusFilterOperatorscan only the required doc ids when present (top-K and threshold modes) — top-K is selected from the allowed set directly, never computed physically and intersected afterwards. Readers that cannot honor the filter are routed to the exact allowed-doc scan at plan time.VECTOR_SIMILARITY_RADIUSwas already correct without this (its candidate-saturation fallback guarantees completeness), but is included so obsolete rows stop wasting its candidate budget and triggering the expensive saturation fallback early.upsertRequiredDocIdsCardinality, the search mode actually executed (FILTER_THEN_ANN/EXACT_SCAN), and a clearupsert_snapshot_*fallback reason for non-filter-aware readers.2. Filter-aware mutable HNSW (pinot-segment-local)
MutableVectorIndexnow implementsFilterAwareVectorIndexReader, so consuming segments use filtered ANN instead of an exact-scan fallback:ScoreDoc.docdirectly and silently assumed Lucene doc ids equal Pinot doc ids — untrue once Lucene merges renumber across commits.SearcherManagernear-real-time view over the writer (refresh-coalescing, reader reused when nothing changed), so uncommitted rows — typically the newest version of a record — are visible. The unfiltered path keeps the cheaper last-committed view, unchanged.BasePinotDocIdBitmapFilterQuery, now shared with the immutableHnswVectorIndexReader's filter query so the correctness-sensitive scaffolding cannot drift).FilterAwareVectorIndexReaderJavadoc now documents the strict contract implementations sign up for: filtered results MUST be a subset of the bitmap, never heuristically degraded — upsert correctness depends on it.3. Vector-column encoding validation (pinot-segment-local)
VectorIndexType.validate()now rejects dictionary-encoded vector columns. Every forward-index read path used by vector search (exact-scan fallback, exact rerank, distance-threshold and radius refinement) callsgetFloatMV, which dictionary-encoded MV readers do not implement — such configs passed validation but failed at query time.A table config with a vector index on a column not declared
encodingType: RAW(dictionary encoding is the default) was previously accepted; after this change its next table-config create/update fails validation. Such tables were already broken for the rerank/threshold/exact-scan/radius query paths. Migration: declareencodingType: RAW(or add the column tonoDictionaryColumns) and reload segments. All in-tree vector tables already use RAW.Other behavior changes to be aware of:
VectorIndexReaderplugins that are not filter-aware now take an exact allowed-doc scan on upsert tables (logged at DEBUG with anupsert_snapshot_*reason), and fail loudly if no forward index exists where they previously returned upsert-inconsistent unfiltered ANN results. All built-in readers (immutable HNSW/IVF_FLAT/IVF_PQ/IVF_ON_DISK and now mutable HNSW) are filter-aware.Testing
All deterministic — no reliance on probabilistic ANN recall (fixtures place obsolete rows physically nearest, with distinct distances).
custom/VectorUpsertTableTest, shared suite — no dedicated cluster): FULL upsert, one Kafka partition, 2-D vectors, stable producer keys, polling-based waits. Covers (1) all records in one consuming segment (NRT filtered mutable HNSW) and (2) obsolete rows sealed into an immutable segment with the new versions in the next consuming segment (filtered immutable HNSW, cross-segment invalidation); both query engines;skipUpsert=truecontrol proving obsolete rows are physically nearest; agreement with the exact scalar-distance query. Pre-existingVectorTest(19) andIvfPqVectorRealtimeTest(6) pass unchanged.Notes / follow-ups