Fix stale docId attribution in ExpressionScanDocIdIterator under look-ahead projection operators - #19245
Open
noob-se7en wants to merge 1 commit into
Open
Conversation
…-ahead projection operators ExpressionScanDocIdIterator resolves match positions through a scratch buffer shared with its doc-id source; every nextBlock() call on the source overwrites it. A pluggable ProjectionOperator (ProjectionOperatorUtils) that pulls multiple blocks ahead invalidates the buffer before earlier blocks are evaluated, silently attributing matches to the wrong docIds. Emit from the projection block's own docIds instead, document the buffer-reuse contract, and add a regression test.
noob-se7en
marked this pull request as draft
August 13, 2026 08:46
noob-se7en
added
bug
noob-se7en
marked this pull request as ready for review
August 13, 2026 08:54
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19245 +/- ##
============================================
+ Coverage 66.94% 66.96% +0.01%
Complexity 1423 1423
============================================
Files 3452 3453 +1
Lines 218619 218860 +241
Branches 34742 34788 +46
============================================
+ Hits 146363 146552 +189
- Misses 60564 60597 +33
- Partials 11692 11711 +19
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:
|
Jackie-Jiang
requested review from
yashmayya
and
a balanced review from Copilot
August 13, 2026 18:33
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes stale document-ID attribution when pluggable projection operators pull blocks ahead of processing.
Changes:
- Uses projection-block document IDs when emitting predicate matches.
- Documents scratch-buffer reuse requirements.
- Adds regression coverage for look-ahead projection and null handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ExpressionScanDocIdIterator.java |
Attributes matches using block-aligned document IDs. |
DocIdSetBlock.java |
Documents scratch-state lifetime. |
DocIdSetOperator.java |
Documents thread-local buffer reuse. |
BitmapDocIdSetOperator.java |
Documents buffer invalidation behavior. |
ExpressionScanDocIdIteratorTest.java |
Tests default and pull-ahead projection paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yashmayya
approved these changes
Aug 13, 2026
| /// every `nextBlock()` call on the source overwrites it. If the iterator resolves match positions through that shared | ||
| /// buffer instead of the docIds carried by the projection block being processed, all blocks except the last one in a | ||
| /// look-ahead window get their matches attributed to the last block's docIds, silently returning wrong results. | ||
| public class ExpressionScanDocIdIteratorTest { |
Contributor
There was a problem hiding this comment.
Maybe add one case for an MV type?
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
ExpressionScanDocIdIteratorresolves match positions through a scratch docId buffer shared with the doc-id source operators it creates: everynextBlock()call on the source overwrites the buffer, andprocessProjectionBlockemitted matches viamatchingDocIds.add(_docIdBuffer[i]).This is only correct while blocks are pulled and processed in lock-step — which the default
ProjectionOperatorhappens to guarantee, but no contract documents. Since #11291 the projection operator is pluggable viaProjectionOperatorUtils; an implementation that pulls several blocks from the doc-id source ahead of consumption (e.g. to overlap remote storage reads with evaluation) leaves the buffer holding the last pulled batch while earlier batches are evaluated. Values and predicate verdicts stay correct (each block fetches values through its own docId copy), but matches get attributed to the wrong docIds — silently wrong results (phantom rows, undercounts from collisions) for any expression predicate AND-ed with an index-based filter.Fix
projectionBlock.getDocIds(), which is position-aligned with the fetched values by construction. With the default operator the block's docIds alias the scratch buffer, so behavior and performance there are unchanged.DocIdSetBlock,BitmapDocIdSetOperator, andDocIdSetOperator: block contents may be invalidated by the nextnextBlock()call; consumers holding blocks across pulls must copy.ProjectionOperatorregistered viaProjectionOperatorUtils; fails before the fix with matches attributed to the last batch's docIds, passes after. Covers the plain, null-handling-enabled, andPredicateEvaluationResult.NULLemission paths, each under both the default and the pull-ahead operator.Impact