Skip to content

Commit 2ddf97a

Browse files
authored
[fix](ann-index) Fix ANN range search state leakage and incorrect slot index tracking. (#63666)
Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note 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. ### Check List (For Author) - 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 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
1 parent 9688e57 commit 2ddf97a

12 files changed

Lines changed: 495 additions & 58 deletions

be/src/exprs/vectorized_fn_call.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,8 @@ Status VectorizedFnCall::evaluate_ann_range_search(
557557
const std::vector<ColumnId>& idx_to_cid,
558558
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
559559
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
560-
bool enable_result_cache) {
560+
bool enable_result_cache, AnnRangeSearchEvaluationResult& evaluation_result) {
561+
evaluation_result = {};
561562
if (range_search_runtime.is_ann_range_search == false) {
562563
return Status::OK();
563564
}
@@ -566,8 +567,8 @@ Status VectorizedFnCall::evaluate_ann_range_search(
566567
range_search_runtime.to_string());
567568
size_t origin_num = row_bitmap.cardinality();
568569

569-
int idx_in_block = static_cast<int>(range_search_runtime.src_col_idx);
570-
DCHECK(idx_in_block < idx_to_cid.size())
570+
const auto idx_in_block = range_search_runtime.src_col_idx;
571+
DCHECK_LT(idx_in_block, idx_to_cid.size())
571572
<< "idx_in_block: " << idx_in_block << ", idx_to_cid.size(): " << idx_to_cid.size();
572573

573574
ColumnId src_col_cid = idx_to_cid[idx_in_block];
@@ -649,6 +650,7 @@ Status VectorizedFnCall::evaluate_ann_range_search(
649650
row_bitmap = *result.roaring;
650651

651652
// Process virtual column
653+
bool dist_fulfilled = false;
652654
if (range_search_runtime.dst_col_idx >= 0) {
653655
// Prepare materialization if we can use result from index.
654656
// Typical situation: range search and operator is LE or LT.
@@ -672,7 +674,7 @@ Status VectorizedFnCall::evaluate_ann_range_search(
672674
}
673675
virtual_column_iterator->prepare_materialization(std::move(distance_col),
674676
std::move(result.row_ids));
675-
_virtual_column_is_fulfilled = true;
677+
dist_fulfilled = true;
676678
} else {
677679
// Whether the ANN index should have produced distance depends on metric and operator:
678680
// - L2: distance is produced for LE/LT; not produced for GE/GT
@@ -686,17 +688,17 @@ Status VectorizedFnCall::evaluate_ann_range_search(
686688
// If we expected distance but didn't get it, assert in debug to catch logic errors.
687689
DCHECK(!should_have_distance) << "Expected distance from ANN index but got none";
688690
#endif
689-
_virtual_column_is_fulfilled = false;
690691
}
691692
} else {
692693
// Dest is not virtual column.
693-
_virtual_column_is_fulfilled = true;
694+
dist_fulfilled = true;
694695
}
695696

696-
_has_been_executed = true;
697+
evaluation_result.executed = true;
698+
evaluation_result.dist_fulfilled = dist_fulfilled;
697699
VLOG_DEBUG << fmt::format(
698700
"Ann range search filtered {} rows, origin {} rows, virtual column is full-filled: {}",
699-
origin_num - row_bitmap.cardinality(), origin_num, _virtual_column_is_fulfilled);
701+
origin_num - row_bitmap.cardinality(), origin_num, dist_fulfilled);
700702

701703
ann_index_stats = *stats;
702704
return Status::OK();

be/src/exprs/vectorized_fn_call.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class VectorizedFnCall : public VExpr {
9292
const std::vector<ColumnId>& idx_to_cid,
9393
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
9494
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
95-
bool enable_result_cache) override;
95+
bool enable_result_cache, AnnRangeSearchEvaluationResult& result) override;
9696

9797
void prepare_ann_range_search(const doris::VectorSearchUserParams& params,
9898
segment_v2::AnnRangeSearchRuntime& runtime,

be/src/exprs/vexpr.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,9 @@ Status VExpr::evaluate_ann_range_search(
10381038
const std::vector<std::unique_ptr<segment_v2::IndexIterator>>& index_iterators,
10391039
const std::vector<ColumnId>& idx_to_cid,
10401040
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
1041-
roaring::Roaring& row_bitmap, AnnIndexStats& ann_index_stats, bool enable_result_cache) {
1041+
roaring::Roaring& row_bitmap, AnnIndexStats& ann_index_stats, bool enable_result_cache,
1042+
AnnRangeSearchEvaluationResult& result) {
1043+
result = {};
10421044
return Status::OK();
10431045
}
10441046

@@ -1056,14 +1058,6 @@ void VExpr::prepare_ann_range_search(const doris::VectorSearchUserParams& params
10561058
}
10571059
}
10581060

1059-
bool VExpr::ann_range_search_executedd() {
1060-
return _has_been_executed;
1061-
}
1062-
1063-
bool VExpr::ann_dist_is_fulfilled() const {
1064-
return _virtual_column_is_fulfilled;
1065-
}
1066-
10671061
Status VExpr::execute_filter(VExprContext* context, const Block* block,
10681062
uint8_t* __restrict result_filter_data, size_t rows, bool accept_null,
10691063
bool* can_filter_all) const {

be/src/exprs/vexpr.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,15 @@ struct AnnRangeSearchRuntime;
7979

8080
using Selector = IColumn::Selector;
8181

82+
struct AnnRangeSearchEvaluationResult {
83+
// Indicates whether the expr row_bitmap has been updated.
84+
bool executed = false;
85+
// Indicates whether the virtual column is fulfilled.
86+
// NOTE, if there is no virtual column in the expr tree, and expr
87+
// is evaluated by ann index, this flag is still true.
88+
bool dist_fulfilled = false;
89+
};
90+
8291
class VExpr {
8392
public:
8493
// resize inserted param column to make sure column size equal to block.rows() and return param column index
@@ -346,7 +355,7 @@ class VExpr {
346355
const std::vector<ColumnId>& idx_to_cid,
347356
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
348357
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
349-
bool enable_result_cache);
358+
bool enable_result_cache, AnnRangeSearchEvaluationResult& result);
350359

351360
// Prepare the runtime for ANN range search.
352361
// AnnRangeSearchRuntime is used to store the runtime information of ann range search.
@@ -356,10 +365,6 @@ class VExpr {
356365
segment_v2::AnnRangeSearchRuntime& range_search_runtime,
357366
bool& suitable_for_ann_index);
358367

359-
bool ann_range_search_executedd();
360-
361-
bool ann_dist_is_fulfilled() const;
362-
363368
virtual uint64_t get_digest(uint64_t seed) const;
364369

365370
protected:
@@ -442,13 +447,6 @@ class VExpr {
442447
// ensuring uniqueness during index traversal
443448
uint32_t _index_unique_id = 0;
444449
bool _enable_inverted_index_query = true;
445-
446-
// Indicates whether the expr row_bitmap has been updated.
447-
bool _has_been_executed = false;
448-
// Indicates whether the virtual column is fulfilled.
449-
// NOTE, if there is no virtual column in the expr tree, and expr
450-
// is evaluated by ann index, this flag is still true.
451-
bool _virtual_column_is_fulfilled = false;
452450
};
453451

454452
// NOLINTBEGIN(readability-function-size)

be/src/exprs/vexpr_context.cpp

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "core/block/columns_with_type_and_name.h"
3030
#include "core/column/column.h"
3131
#include "core/column/column_const.h"
32+
#include "exec/common/util.hpp"
3233
#include "exprs/function_context.h"
3334
#include "exprs/vexpr.h"
3435
#include "runtime/runtime_state.h"
@@ -440,41 +441,57 @@ Status VExprContext::evaluate_ann_range_search(
440441
const std::unordered_map<VExprContext*, std::unordered_map<ColumnId, VExpr*>>&
441442
common_expr_to_slotref_map,
442443
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
443-
bool enable_result_cache) {
444+
bool enable_result_cache, bool* ann_range_search_executed) {
445+
if (ann_range_search_executed != nullptr) {
446+
*ann_range_search_executed = false;
447+
}
444448
if (_root == nullptr) {
445449
return Status::OK();
446450
}
447451

452+
AnnRangeSearchEvaluationResult evaluation_result;
448453
RETURN_IF_ERROR(_root->evaluate_ann_range_search(
449454
_ann_range_search_runtime, cid_to_index_iterators, idx_to_cid, column_iterators,
450-
row_bitmap, ann_index_stats, enable_result_cache));
455+
row_bitmap, ann_index_stats, enable_result_cache, evaluation_result));
451456

452-
if (!_root->ann_range_search_executedd()) {
457+
if (!evaluation_result.executed) {
453458
return Status::OK();
454459
}
460+
if (ann_range_search_executed != nullptr) {
461+
*ann_range_search_executed = true;
462+
}
463+
464+
DCHECK(_index_context != nullptr);
465+
_index_context->set_index_result_for_expr(
466+
_root.get(),
467+
segment_v2::InvertedIndexResultBitmap(std::make_shared<roaring::Roaring>(row_bitmap),
468+
std::make_shared<roaring::Roaring>()));
455469

456-
if (!_root->ann_dist_is_fulfilled()) {
470+
if (!evaluation_result.dist_fulfilled) {
457471
// Do not perform index scan in this case.
458472
return Status::OK();
459473
}
460474

461-
auto src_col_idx = _ann_range_search_runtime.src_col_idx;
475+
DCHECK_LT(_ann_range_search_runtime.src_col_idx, idx_to_cid.size());
476+
const auto src_col_idx = cast_set<int>(_ann_range_search_runtime.src_col_idx);
477+
const auto src_col_key = cast_set<ColumnId>(_ann_range_search_runtime.src_col_idx);
462478
auto slot_ref_map_it = common_expr_to_slotref_map.find(this);
463479
if (slot_ref_map_it == common_expr_to_slotref_map.end()) {
464480
return Status::OK();
465481
}
466482
auto& slot_ref_map = slot_ref_map_it->second;
467-
ColumnId cid = idx_to_cid[src_col_idx];
468-
if (slot_ref_map.find(cid) == slot_ref_map.end()) {
483+
auto slot_ref_it = slot_ref_map.find(src_col_key);
484+
if (slot_ref_it == slot_ref_map.end()) {
469485
return Status::OK();
470486
}
471-
const VExpr* slot_ref_expr_addr = slot_ref_map.find(cid)->second;
472-
_index_context->set_true_for_index_status(slot_ref_expr_addr, idx_to_cid[cid]);
487+
const VExpr* slot_ref_expr_addr = slot_ref_it->second;
488+
_index_context->set_true_for_index_status(slot_ref_expr_addr, src_col_idx);
473489

474490
VLOG_DEBUG << fmt::format(
475491
"Evaluate ann range search for expr {}, src_col_idx {}, cid {}, row_bitmap "
476492
"cardinality {}",
477-
_root->debug_string(), src_col_idx, cid, row_bitmap.cardinality());
493+
_root->debug_string(), src_col_idx, idx_to_cid[_ann_range_search_runtime.src_col_idx],
494+
row_bitmap.cardinality());
478495
return Status::OK();
479496
}
480497

be/src/exprs/vexpr_context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ class VExprContext {
389389
const std::unordered_map<VExprContext*, std::unordered_map<ColumnId, VExpr*>>&
390390
common_expr_to_slotref_map,
391391
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
392-
bool enable_result_cache);
392+
bool enable_result_cache, bool* ann_range_search_executed);
393393

394394
uint64_t get_digest(uint64_t seed) const;
395395

be/src/exprs/virtual_slot_ref.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,11 +235,9 @@ Status VirtualSlotRef::evaluate_ann_range_search(
235235
const std::vector<ColumnId>& idx_to_cid,
236236
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
237237
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
238-
bool enable_result_cache) {
238+
bool enable_result_cache, AnnRangeSearchEvaluationResult& result) {
239239
return _virtual_column_expr->evaluate_ann_range_search(
240240
range_search_runtime, cid_to_index_iterators, idx_to_cid, column_iterators, row_bitmap,
241-
ann_index_stats, enable_result_cache);
242-
243-
return Status::OK();
241+
ann_index_stats, enable_result_cache, result);
244242
}
245243
} // namespace doris

be/src/exprs/virtual_slot_ref.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class VirtualSlotRef MOCK_REMOVE(final) : public VExpr {
108108
const std::vector<ColumnId>& idx_to_cid,
109109
const std::vector<std::unique_ptr<segment_v2::ColumnIterator>>& column_iterators,
110110
roaring::Roaring& row_bitmap, segment_v2::AnnIndexStats& ann_index_stats,
111-
bool enable_result_cache) override;
111+
bool enable_result_cache, AnnRangeSearchEvaluationResult& result) override;
112112

113113
#ifdef BE_TEST
114114
// Test-only setter methods for unit testing

be/src/storage/segment/segment_iterator.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,10 +1366,14 @@ Status SegmentIterator::_apply_index_expr() {
13661366
for (const auto& expr_ctx : _common_expr_ctxs_push_down) {
13671367
segment_v2::AnnIndexStats ann_index_stats;
13681368
size_t origin_rows = _row_bitmap.cardinality();
1369+
bool ann_range_search_executed = false;
13691370
RETURN_IF_ERROR(expr_ctx->evaluate_ann_range_search(
13701371
_index_iterators, _schema->column_ids(), _column_iterators,
13711372
_common_expr_to_slotref_map, _row_bitmap, ann_index_stats,
1372-
enable_ann_index_result_cache));
1373+
enable_ann_index_result_cache, &ann_range_search_executed));
1374+
if (ann_range_search_executed) {
1375+
_opts.stats->ann_index_range_search_cnt++;
1376+
}
13731377
_opts.stats->rows_ann_index_range_filtered += (origin_rows - _row_bitmap.cardinality());
13741378
_opts.stats->ann_index_load_ns += ann_index_stats.load_index_costs_ns.value();
13751379
_opts.stats->ann_index_range_search_ns += ann_index_stats.search_costs_ns.value();
@@ -1386,14 +1390,6 @@ Status SegmentIterator::_apply_index_expr() {
13861390
_opts.stats->ann_index_range_cache_hits += ann_index_stats.range_cache_hits.value();
13871391
}
13881392

1389-
for (auto it = _common_expr_ctxs_push_down.begin(); it != _common_expr_ctxs_push_down.end();) {
1390-
if ((*it)->root()->ann_range_search_executedd()) {
1391-
_opts.stats->ann_index_range_search_cnt++;
1392-
it = _common_expr_ctxs_push_down.erase(it);
1393-
} else {
1394-
++it;
1395-
}
1396-
}
13971393
return Status::OK();
13981394
}
13991395

0 commit comments

Comments
 (0)