[fix](ann-index) Fix ANN range search state leakage and incorrect slot index tracking.#63965
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Pull request overview
Fixes correctness issues in BE ANN range search pushdown by eliminating execution-state leakage across VExprContext clones and correcting how common-expression index-evaluation status is tracked when scan-block column order differs from storage column ids.
Changes:
- Replace ANN “executed/fulfilled” state stored on shared
VExprroots with a per-evaluation result object propagated viaevaluate_ann_range_search(...). - Track ANN range-search results in the current segment’s
IndexExecContextand update common-expression index status using source column index (scan-block) rather than storageColumnId. - Add BE unit tests and regression tests covering mixed indexed/non-indexed segments and remapped schema/index-status cases.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| regression-test/suites/ann_index_p0/ann_range_search_source_index_status_regression.groovy | Adds regression coverage to ensure common-expr index status uses source column index so embedding reads can be skipped correctly. |
| regression-test/suites/ann_index_p0/ann_range_search_pushdown_regression.groovy | Adds regression coverage for mixed indexed/non-indexed segments ensuring ANN execution state doesn’t leak and profile counter is correct. |
| be/test/storage/index/ann/ann_range_search_test.cpp | Extends unit tests to validate no cross-clone state leakage and correct slot-map/index-status updates. |
| be/src/storage/segment/segment_iterator.cpp | Counts ANN range-search executions per evaluation call and removes reliance on shared-root state. |
| be/src/exprs/virtual_slot_ref.h | Updates ANN range-search virtual API signature to return per-call evaluation result. |
| be/src/exprs/virtual_slot_ref.cpp | Propagates new ANN evaluation result through virtual slot ref. |
| be/src/exprs/vexpr.h | Introduces AnnRangeSearchEvaluationResult and updates virtual method signature; removes shared-root execution state fields. |
| be/src/exprs/vexpr.cpp | Removes legacy state accessors and adapts base method signature. |
| be/src/exprs/vexpr_context.h | Extends context API to return per-call “executed” via out-param and support result caching flag. |
| be/src/exprs/vexpr_context.cpp | Uses per-call evaluation result, stores bitmap in IndexExecContext, and fixes source-index-based slot-map lookup / index-status update. |
| be/src/exprs/vectorized_fn_call.h | Updates ANN range-search override signature to return per-call evaluation result. |
| be/src/exprs/vectorized_fn_call.cpp | Sets per-call executed/fulfilled result instead of mutating shared-root state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "exec/common/util.hpp" | ||
| #include "exprs/function_context.h" |
e48e0b8 to
551cc28
Compare
|
run buildall |
…t index tracking. (apache#63666) Issue Number: close #xxx Related PR: #xxx Problem Summary: ANN range search execution state was stored on shared VExpr roots. VExprContext clones share the root expression, so a segment that executed ANN range search could leak that state into another segment without an ANN index and incorrectly remove the common expression. ANN range search also mixed schema column indexes with storage column ids when updating common expression index status, so remapped schemas failed to mark the source slot expression as evaluated. This patch returns ANN execution state through the current evaluation call, stores ANN root bitmap in the current segment IndexContext, and updates slot index status by source column index. - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
551cc28 to
0eecfe6
Compare
|
run buildall |
|
skip buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
Cherrypick #63666
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)