[fix](inverted-index) Avoid global null bitmap in sub-reader context#65549
Open
hoshinojyunn wants to merge 1 commit into
Open
[fix](inverted-index) Avoid global null bitmap in sub-reader context#65549hoshinojyunn wants to merge 1 commit into
hoshinojyunn wants to merge 1 commit into
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Multi-segment collection creates a QueryExecutionContext for each CLucene sub-reader. Copying the root NULL bitmap resolver into that context lets nullable boolean scorers interpret a table-level NULL bitmap with local docids. For example, consider two sub-readers with two documents each. In the first sub-reader, local doc 0 maps to global doc 0 and is NULL. In the second, local doc 0 maps to global doc 2. The root NULL bitmap is therefore {0}. For NOT PREFIX(title, "missing"), the second sub-reader incorrectly treats global bitmap entry 0 as its local doc 0, excludes global doc 2, and contributes only global doc 3. The collected TRUE bitmap becomes {1,3}; masking the actual root NULL bitmap {0} cannot restore doc 2, while the correct result is {1,2,3}. Keep the resolver on the root context only. The collector test covers this two-sub-reader case and verifies that the final NOT result retains global doc 2.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh --run --filter=MultiSegmentCollectorTest.* -j16
- Behavior changed: Yes (fixes incorrect multi-segment boolean query results with NULLs)
- Does this need documentation: No
hoshinojyunn
requested review from
airborne12,
csun5285 and
eldenmoon
as code owners
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
hoshinojyunn
changed the title
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29274 ms |
Contributor
TPC-DS: Total hot run time: 180045 ms |
Contributor
ClickBench: Total hot run time: 24.99 s |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review result: request changes.
I found one correctness issue. The PR fixes the full doc-set regression for global NULL bitmaps in sub-reader contexts, but the same helper is used by descending scored top-k before the later global NULL mask. That can drop valid non-NULL rows after top-k truncation.
Checkpoint conclusions: goal is mostly addressed for doc-set collection, but not for the scored top-k parallel path; the code change is small and focused; I did not find new concurrency, lifecycle, config, persistence, storage-format, or cache-handle lifetime issues; the main missing coverage is a scored descending top-k regression with nullable/NOT semantics. No additional user focus was provided.
Validation: static review only per the review prompt. I did not run builds or tests. The local checkout lacks the base commit and thirdparty/protoc, so whitespace checking was limited to the checked-out HEAD commit and produced no reported issues.
| @@ -171,7 +171,6 @@ inline QueryExecutionContext create_segment_context(const QueryExecutionContext& | |||
| } | |||
|
|
|||
| seg_ctx.binding_fields = original_ctx.binding_fields; | |||
Contributor
There was a problem hiding this comment.
This fixes the full doc-set case, but the same segment context is used by collect_multi_segment_top_k(). In that path we build scorers from seg_ctx, keep only k docs per segment and then globally, and only later does FunctionSearch rebuild a global scorer and call mask_out_null(). Because this context no longer has a resolver, nullable NOT/boolean scorers treat NULL rows as ordinary matches during top-k selection; after the later mask removes those rows, the next non-NULL candidates have already been discarded. A concrete shape is top_k=3 over docs 0..3 where doc 0 is NULL for the field and the query is NOT missing: top-k can retain 0,1,2 on equal scores, the global mask drops 0, and doc 3 is never reconsidered. Please either filter/mark NULL rows before top-k truncation using segment-local null bitmaps, or apply the original global NULL mask before enforcing the final k, and add a scored descending top-k regression.
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
2 participants
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.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
create_segment_context()creates aQueryExecutionContextfor each CLucene sub-reader. The root NULL bitmap resolver uses table-level global docids, while the child scorer evaluates its boolean expression with sub-reader-local docids. Copying that resolver into the child context mixes these coordinate systems.Reproduction:
MultiReaderwith two sub-readers, each containing two documents.0to NULL. The root NULL bitmap is{0}.0maps to global doc0; in the second sub-reader, local doc0maps to global doc2.NOT PREFIX(title, "missing").Incorrect result before this fix:
{0}.0as local doc0rather than global doc0.2and contributes only global doc3.{1,3}.{0}cannot restore the incorrectly omitted doc2.Expected result:
{0,1,2,3}.{0}produces{1,2,3}.Fix:
null_resolveron the root execution context.create_segment_context().NOTcase and verifies global doc2is retained.Release note
None
Check List (For Author)
Test
./run-be-ut.sh --run --filter=MultiSegmentCollectorTest.* -j16Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)