github-actions[bot] commented on code in PR #64377:
URL: https://github.com/apache/doris/pull/64377#discussion_r3576746067
##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -1061,6 +1074,80 @@ Status SegmentIterator::_apply_ann_topn_predicate() {
return Status::OK();
}
+bool SegmentIterator::_enable_ann_topn_predicate_prefilter() const {
+ return !_opts.runtime_state ||
+
!_opts.runtime_state->query_options().__isset.enable_ann_topn_predicate_prefilter
||
+
_opts.runtime_state->query_options().enable_ann_topn_predicate_prefilter;
+}
+
+Status SegmentIterator::_eager_filter_predicates_into_bitmap() {
+ // The residual column predicates (those not resolvable by
zonemap/inverted/bitmap index, hence
+ // not yet reflected in _row_bitmap) are evaluated here over the candidate
rows, and the
+ // survivors are intersected back into _row_bitmap. The narrowed bitmap is
then fed to the ANN
+ // index as an IDSelector (see
AnnTopNRuntime::evaluate_vector_ann_search), so a predicated
+ // TopN query keeps using the index instead of degrading to an O(N)
brute-force distance scan.
+ //
+ // This runs before the main scan loop sets up _range_iter / _block_rowids
/
+ // _current_return_columns (see _lazy_init), so it allocates the predicate
columns and drives a
+ // local pass with the same _read_columns_by_index + predicate-evaluation
primitives the main
+ // loop uses. All of those members are re-initialized by the main loop
afterwards (_range_iter
+ // is recreated over the narrowed _row_bitmap right after
_apply_ann_topn_predicate returns).
+ if (!_is_need_vec_eval && !_is_need_short_eval) {
+ // has_column_predicate was true but every predicate was already
resolved via index;
+ // there is nothing left to evaluate per row.
+ return Status::OK();
+ }
+
+ _current_return_columns.resize(_schema->columns().size());
+ for (auto cid : _predicate_column_ids) {
+ auto storage_column_type = _storage_name_and_type[cid].second;
+ RETURN_IF_CATCH_EXCEPTION(_current_return_columns[cid] =
Schema::get_predicate_column_ptr(
+ storage_column_type,
_opts.io_ctx.reader_type));
+ _current_return_columns[cid]->set_rowset_segment_id(
+ {_segment->rowset_id(), _segment->id()});
+ }
+
+ if (_block_rowids.size() < _opts.block_row_max) {
+ _block_rowids.resize(_opts.block_row_max);
+ }
+
+ _range_iter.reset(new BitmapRangeIterator(_row_bitmap));
+ roaring::Roaring survivors;
+ while (true) {
+ // The predicate columns are reused across batches, so clear them
before each read just
+ // like the main scan loop does in _init_current_block. Without this,
_read_columns_by_index
+ // appends onto the previous batch's data and predicate evaluation
reads stale rows once the
+ // candidate set spans more than one block_row_max batch.
+ for (auto cid : _predicate_column_ids) {
+ _current_return_columns[cid]->clear();
+ }
+ _selected_size = 0;
+ RETURN_IF_ERROR(_read_columns_by_index(_opts.block_row_max,
_selected_size));
+ if (_selected_size == 0) {
+ break;
+ }
+ // Mirror the normal scan loop's post-read fixups
(_next_batch_internal): hidden columns
+ // like __DORIS_VERSION_COL__ and row-binlog LSN/TSO columns are
stored as placeholders on
+ // disk and only become their real, predicate-visible values after
these substitutions.
+ // Skipping them here would let a predicate on one of those columns
narrow _row_bitmap
+ // using placeholder data, diverging from what the normal/brute-force
path would evaluate.
+ _replace_version_col_if_needed(_predicate_column_ids, _selected_size);
+ _update_lsn_col_if_needed(_predicate_column_ids, _selected_size);
Review Comment:
This added call is unresolved in the current source. `SegmentIterator` only
declares/defines `_replace_version_col_if_needed(...)` and
`_update_tso_col_if_needed(...)`; a repository search shows
`_update_lsn_col_if_needed(...)` exists only at this call site, and the normal
scan path still applies only the version/TSO fixups after
predicate/common/non-predicate reads. As written this will fail to compile
before the prefilter path can run. Please remove the LSN call and adjust the
comment to match the existing helpers, or add a real helper and apply it
consistently to the normal read paths too if LSN needs a new read-time
substitution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
