github-actions[bot] commented on code in PR #66052:
URL: https://github.com/apache/doris/pull/66052#discussion_r3767349002
##########
be/src/exprs/function/function_search.cpp:
##########
@@ -734,6 +787,93 @@ Status FunctionSearch::build_leaf_query(const
TSearchClause& clause,
*binding_key = binding.binding_key;
}
+ if (binding.use_snii_native_reader()) {
+ DORIS_CHECK(binding.inverted_reader != nullptr);
+ // The SNII reader answers a clause directly from a query type: it
tokenizes the value
+ // itself and owns the matching operator, so unlike the CLucene path
below there is no
+ // query tree to assemble here. RANGE and LIST reach the same TERM
fallback the CLucene
+ // path uses, because neither is implemented there either.
+ InvertedIndexQueryType snii_query_type = (clause_type == "RANGE" ||
clause_type == "LIST")
+ ?
InvertedIndexQueryType::EQUAL_QUERY
+ :
clause_type_to_query_type(clause_type);
+
+ if (clause_type == "TERM") {
+ // minimum_should_match ("at least N of M terms") has no SNII
query type: the reader
+ // only knows AND-all (MATCH_ALL_QUERY) or OR-all
(EQUAL_QUERY/MATCH_ANY_QUERY) of the
+ // terms it tokenizes internally, never a partial threshold. The
CLucene path builds
+ // an OccurBooleanQuery for this (function_search.cpp:913-926);
SNII cannot, so refuse
+ // explicitly instead of silently answering a plain OR query.
+ //
+ // But the CLucene path only ever reaches that OccurBooleanQuery
when the field is
+ // analysed (:925's `if (should_analyze)` short-circuits first): a
keyword (non-
+ // analysed) TERM value tokenizes to at most one term, so msm is
never consulted for
+ // it there either -- it is simply ignored. Refusing it here for a
keyword field would
+ // hard-error a query V2/V3 answers fine, so gate the refusal the
same way.
+ if (minimum_should_match > 0 &&
+
inverted_index::InvertedIndexAnalyzer::should_analyzer(binding.index_properties))
{
+ return Status::NotSupported(
+ "SNII native SEARCH does not support
minimum_should_match for TERM "
+ "clauses (got {})",
+ minimum_should_match);
+ }
+ // default_operator selects how a multi-token TERM value combines:
"and" requires
+ // every term (MATCH_ALL_QUERY), "or" -- the default -- requires
any term, which is
+ // already snii_query_type above (EQUAL_QUERY). A single-token
value is unaffected
+ // either way, since the reader special-cases terms.size() == 1
for both query types.
+ if (default_operator == "and") {
+ snii_query_type = InvertedIndexQueryType::MATCH_ALL_QUERY;
+ }
+ } else if (clause_type == "PREFIX" &&
+ !inverted_index::InvertedIndexAnalyzer::should_analyzer(
+ binding.index_properties)) {
+ // FE keeps the trailing '*' in the PREFIX value unstripped
(SearchDslParser.java).
+ // On an analysed field the tokenizer drops it, leaving a clean
single prefix term,
+ // so the default clause_type_to_query_type mapping
(MATCH_PHRASE_PREFIX_QUERY) is
+ // correct as-is. On a keyword (non-analysed) field the whole
string -- '*' included
+ // -- becomes one literal term
(InvertedIndexAnalyzer::get_analyse_result), so
+ // MATCH_PHRASE_PREFIX_QUERY would search for a term that can
never exist. Route
+ // those to WILDCARD_QUERY instead, exactly like the CLucene path's
+ // WildcardQuery(value) for PREFIX
(function_search.cpp:1075-1076): the reader
+ // forwards a WILDCARD_QUERY value unanalysed, so the trailing '*'
works the same way.
+ snii_query_type = InvertedIndexQueryType::WILDCARD_QUERY;
+ }
+
+ auto data_bitmap = std::make_shared<roaring::Roaring>();
+ if (clause_type == "WILDCARD" && value == "*") {
+ data_bitmap->addRange(0, num_rows);
+ } else {
+ // Wildcard patterns carry the analyzer's lower_case semantics;
every other clause
+ // passes its value through untouched, since the reader analyses
it.
+ std::string pattern =
+ clause_type == "WILDCARD"
+ ? normalize_wildcard_pattern(value,
binding.index_properties)
+ : value;
+ Field query_value = Field::create_field<TYPE_STRING>(pattern);
+ RETURN_IF_ERROR(binding.inverted_reader->query(context,
binding.stored_field_name,
+ query_value,
snii_query_type,
+ data_bitmap,
nullptr));
+ // Restore the pre-normalization value for WILDCARD so the trace
still shows what the
+ // caller actually asked for, not just what was sent to the reader.
+ std::string log_suffix =
+ clause_type == "WILDCARD" ? (" (original='" + value +
"')") : std::string();
+ VLOG_DEBUG << "search: SNII clause processed, type=" << clause_type
+ << ", field=" << field_name << ", value='" << pattern
<< "'" << log_suffix;
+ }
+
+ auto null_bitmap = std::make_shared<roaring::Roaring>();
+ if (binding.inverted_reader->has_null()) {
+ segment_v2::InvertedIndexQueryCacheHandle null_bitmap_cache_handle;
+ RETURN_IF_ERROR(binding.inverted_reader->read_null_bitmap(
+ context, &null_bitmap_cache_handle, nullptr));
+ auto cached_null_bitmap = null_bitmap_cache_handle.get_bitmap();
+ DORIS_CHECK(cached_null_bitmap != nullptr);
Review Comment:
[P1] Carry real SNII scores through the query-v2 bridge.
`SniiIndexReader::query()` eagerly writes BM25 values for scoring clauses, but
this `BitSetQuery` exposes a scorer fixed at `1.0`. Descending early top-k
therefore selects rows using constants instead of BM25; ordinary collection
adds `1.0` again to scores already stored, and TERM/EXACT receive only the
invented constant. Provide a scorer that exposes the native per-document values
(or move scoring wholly into query-v2), collect each score once, and add
SNII/V3 score and `ORDER BY score DESC LIMIT` parity tests.
##########
be/src/io/cache/cached_remote_file_reader.cpp:
##########
@@ -466,6 +478,7 @@ void launch_s3_race(std::shared_ptr<RaceState> race, size_t
empty_start, size_t
if (st.ok() && race->winner < 0) {
Review Comment:
[P2] Count a successful losing S3 hedge as physical remote I/O. Once
launched, this task performs a real object-store GET, but the size is retained
only while S3 can become the winner and is merged only on the S3-winner branch.
A slower successful S3 leg therefore consumes remote bytes while
`InvertedIndexRemotePhysicalReadBytes` reports none. Account completion through
a lifetime-safe owner (or cancel/wait for the loser) and add a
peer-wins-after-S3-starts metric test.
##########
be/src/storage/index/index_file_reader.cpp:
##########
@@ -155,6 +210,69 @@ Result<std::unique_ptr<DorisCompoundReader,
DirectoryDeleter>> IndexFileReader::
int64_t index_id, const std::string& index_suffix, const
io::IOContext* io_ctx) const {
std::unique_ptr<DorisCompoundReader, DirectoryDeleter> compound_reader;
+ if (_storage_format == InvertedIndexStorageFormatPB::SNII) {
+ // A blob logical index is a named-sub-file table over the container,
and
+ // a compound reader is a named-sub-file table over a stream -- the
same
+ // shape. The offsets recorded in the directory are ABSOLUTE container
+ // offsets, exactly like a V2 compound entry, so the sub-files need no
+ // rebasing and DorisCompoundReader is reused unchanged.
+ const auto index_file_path =
+
InvertedIndexDescriptor::get_index_file_path_v2(_index_path_prefix);
+ EntriesType entries;
+ int64_t container_size = 0;
+ // The lock spans every use of _snii_segment_reader state, not just the
+ // lookup: `entry` points into the reader's decoded directory, and
every
+ // other SNII accessor on this class holds the lock across the whole
use.
+ // Narrowing it here would make this the one site whose safety rests on
+ // "the segment reader is never reset after init" rather than on the
lock.
+ {
+ std::shared_lock<std::shared_mutex> lock(_mutex);
+ if (_snii_segment_reader == nullptr) {
+ return
ResultError(Status::Error<ErrorCode::INVERTED_INDEX_FILE_NOT_FOUND>(
+ "SNII index file {} is not opened", index_file_path));
+ }
+ const doris::snii::format::LogicalIndexMetadataRef* entry =
nullptr;
+
RETURN_IF_ERROR_RESULT(_snii_segment_reader->blob_entry(cast_set<uint64_t>(index_id),
+
index_suffix, &entry));
+ DORIS_CHECK(entry != nullptr);
+ // Only an ANN index is served through a CLucene directory. A BKD
blob
+ // has its own reader and must not be reachable this way, or a
caller
+ // would get a directory over bytes no CLucene code can parse.
+ if (entry->kind != doris::snii::format::LogicalIndexKind::kAnn) {
+ return
ResultError(Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>(
+ "SNII logical index {} is not an ANN blob; it has no
CLucene directory",
+ index_id));
+ }
+ // Blob extents were bounded against the container at open time
Review Comment:
[P1] Validate named blob content before exposing it. The compound writer
persists `crc32c` for every ANN/BKD named file, but this adapter drops it when
constructing `ReaderFileEntry`; the BKD adapter likewise keeps only extents. No
production reader consumes `NamedBlobFileRef::crc32c`. In particular, lazy
`bkd_data` leaves have structural checks but no payload CRC, so a decodable
mutation can change the result bitmap. Please enforce integrity before these
bytes affect queries (with per-leaf granularity for cold BKD so reads stay
lazy) and add corruption coverage.
##########
be/src/storage/index/snii/format/sampled_term_index.cpp:
##########
@@ -0,0 +1,189 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "storage/index/snii/format/sampled_term_index.h"
+
+#include <algorithm>
+
+#include "storage/index/snii/encoding/byte_source.h"
+#include "storage/index/snii/encoding/section_framer.h"
+
+namespace doris::snii::format {
+
+namespace {
+
+// Longest common prefix length of term and prev (front coding primitive,
consistent with dict_entry).
+uint32_t common_prefix_len(std::string_view term, std::string_view prev) {
+ uint32_t n = 0;
+ const uint32_t lim = static_cast<uint32_t>(std::min(term.size(),
prev.size()));
+ while (n < lim && term[n] == prev[n]) ++n;
+ return n;
+}
+
+// Write a front-coded term key (prefix_len + suffix_len + suffix).
+void write_term_key(std::string_view term, std::string_view prev, ByteSink*
sink) {
+ const uint32_t prefix = common_prefix_len(term, prev);
+ const std::string_view suffix = term.substr(prefix);
+ sink->put_varint32(prefix);
+ sink->put_varint32(static_cast<uint32_t>(suffix.size()));
+ sink->put_bytes(Slice(suffix));
+}
+
+// Read a front-coded term key and reconstruct it into out from prev + suffix.
+Status read_term_key(ByteSource* src, std::string_view prev, std::string* out)
{
+ uint32_t prefix = 0;
+ uint32_t suffix_len = 0;
+ RETURN_IF_ERROR(src->get_varint32(&prefix));
+ RETURN_IF_ERROR(src->get_varint32(&suffix_len));
+ if (prefix > prev.size()) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_FILE_CORRUPTED, false>(
+ "sampled_term_index: prefix_len exceeds prev_term length");
+ }
+ Slice suffix;
+ RETURN_IF_ERROR(src->get_bytes(suffix_len, &suffix));
+ out->assign(prev.substr(0, prefix));
+ out->append(reinterpret_cast<const char*>(suffix.data()), suffix.size());
+ return Status::OK();
+}
+
+} // namespace
+
+void SampledTermIndexBuilder::add_block_first_term(std::string_view
first_term) {
+ first_terms_.emplace_back(first_term);
+}
+
+void SampledTermIndexBuilder::finish(ByteSink* sink) {
+ ByteSink payload;
+ payload.put_varint32(static_cast<uint32_t>(first_terms_.size()));
+ // min_term / max_term are written only when non-empty (== first/last
sample_term).
+ if (!first_terms_.empty()) {
+ write_term_key(first_terms_.front(), std::string_view {}, &payload);
+ write_term_key(first_terms_.back(), std::string_view {}, &payload);
+ std::string_view prev {};
+ for (const auto& t : first_terms_) {
+ write_term_key(t, prev, &payload);
+ prev = t;
+ }
+ }
+ SectionFramer::write(*sink,
static_cast<uint8_t>(SectionType::kSampledTermIndex),
+ payload.view());
+}
+
+namespace {
+
+// Parse n_blocks, min/max (not used directly; consumed for checksum
alignment), and all sample_terms from payload.
+Status parse_payload(Slice payload, std::vector<std::string>* terms) {
+ ByteSource src(payload);
+ uint32_t n_blocks = 0;
+ RETURN_IF_ERROR(src.get_varint32(&n_blocks));
+ if (n_blocks == 0) {
+ if (!src.eof()) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_FILE_CORRUPTED,
false>(
+ "sampled_term_index: empty index contains trailing bytes");
+ }
+ terms->clear();
+ return Status::OK();
+ }
+
+ // min_term / max_term (do not drive binary search directly; must be
consumed to verify structural alignment).
+ std::string min_term;
+ std::string max_term;
+ RETURN_IF_ERROR(read_term_key(&src, std::string_view {}, &min_term));
+ RETURN_IF_ERROR(read_term_key(&src, std::string_view {}, &max_term));
+
+ std::vector<std::string> out;
+ out.reserve(n_blocks);
Review Comment:
[P1] Bound `n_blocks` by the remaining payload before reserving. This
persisted `uint32` is passed directly to `vector<string>::reserve()` after two
independently small strings. A tiny, CRC-valid but malformed section can claim
`UINT32_MAX` and request roughly 100+ GiB of vector capacity before the loop
notices that entries are missing. Derive a conservative maximum from
`src.remaining()` before allocation, as the adjacent dict-block-directory
decoder does, and add a max-count corruption test.
##########
be/src/storage/index/index_file_writer.cpp:
##########
@@ -197,6 +504,29 @@ Result<std::unique_ptr<IndexSearcherBuilder>>
IndexFileWriter::_construct_index_
Status IndexFileWriter::begin_close() {
DCHECK(!_closed) << debug_string();
_closed = true;
+ if (_storage_format == InvertedIndexStorageFormatPB::SNII) {
+ if (_snii_compound_writer == nullptr) {
+ if (_idx_v2_writer == nullptr) {
+ return Status::OK();
+ }
Review Comment:
[P2] Release SNII blob scratch directories on every close exit. Both
`RETURN_IF_ERROR` paths here precede `_release_snii_blob_directories()`, while
`DorisFSDirectory` destruction does not remove its on-disk directory. Repeated
ANN seal/finalize failures can therefore retain scratch trees until tmp cleanup
or restart. The release path also calls a throwing `deleteDirectory()` across
this `Status` API. Please make cleanup non-throwing/RAII and run it on every
terminal path, with fault-injection coverage.
##########
be/src/storage/index/snii/snii_index_writer.cpp:
##########
@@ -0,0 +1,508 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "storage/index/snii/snii_index_writer.h"
+
+#include <CLucene.h>
+
+#include <algorithm>
+#include <cstring>
+#include <string_view>
+#include <utility>
+
+#include "common/cast_set.h"
+#include "common/config.h"
+#include "common/logging.h"
+#include "storage/index/index_file_writer.h"
+#include "storage/index/inverted/analyzer/analyzer.h"
+#include "storage/index/inverted/common_grams/common_grams_key_codec.h"
+#include "storage/index/inverted/query/query_info.h"
+#include "storage/index/inverted/token_filter/common_grams_filter.h"
+#include "storage/index/snii/query/bm25_scorer.h"
+#include "storage/index/snii/writer/global_memory_limiter.h"
+#include "storage/tablet/tablet_schema.h"
+
+namespace doris::segment_v2 {
+namespace {
+
+Status validate_common_grams_metadata_seed(
+ const inverted_index::CommonGramsSegmentMetadata& metadata) {
+ using namespace inverted_index;
+ auto status = validate_common_grams_segment_metadata(metadata);
+ if (!status.ok() || metadata.plain_term_key_version !=
PlainTermKeyVersion::kEscapedV1 ||
+ metadata.common_grams_coverage != CommonGramsCoverage::kComplete ||
+ metadata.common_grams_semantics_version !=
COMMON_GRAMS_SEMANTICS_VERSION_V1 ||
+ metadata.common_grams_key_version != COMMON_GRAMS_KEY_VERSION_V1 ||
+ metadata.scoring_coverage != ScoringCoverage::kComplete ||
+ metadata.scoring_stats_version !=
COMMON_GRAMS_SCORING_STATS_VERSION_V1 ||
+ metadata.norm_semantics_version !=
COMMON_GRAMS_NORM_SEMANTICS_VERSION_V1) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_ANALYZER_ERROR>(
+ "SNII CommonGrams metadata identity seed is incomplete or
incompatible");
+ }
+ return Status::OK();
+}
+
+} // namespace
+
+SniiIndexColumnWriter::SniiIndexColumnWriter(
+ IndexFileWriter* index_file_writer, const TabletIndex* index_meta,
FieldType value_type,
+ std::optional<inverted_index::CommonGramsSegmentMetadata>
common_grams_metadata_seed)
+ : _index_file_writer(index_file_writer),
+ _index_meta(index_meta),
+ _is_char(value_type == FieldType::OLAP_FIELD_TYPE_CHAR),
+ _common_grams_build_enabled(config::enable_common_grams_index_build),
+ _common_grams_metadata_seed(std::move(common_grams_metadata_seed)) {}
+
+Status SniiIndexColumnWriter::init() {
+ _should_analyzer =
+
inverted_index::InvertedIndexAnalyzer::should_analyzer(_index_meta->properties());
+ _has_positions =
get_parser_phrase_support_string_from_properties(_index_meta->properties()) ==
+ INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES;
+ _config = _has_positions ?
::doris::snii::format::IndexConfig::kDocsPositions
+ : ::doris::snii::format::IndexConfig::kDocsOnly;
+ auto ignore_above_value =
+
get_parser_ignore_above_value_from_properties(_index_meta->properties());
+ _ignore_above = cast_set<uint32_t>(std::stoul(ignore_above_value));
+ const auto spill_threshold =
+ static_cast<size_t>(config::inverted_index_ram_buffer_size * 1024
* 1024);
+ _memory_reporter = std::make_unique<::doris::snii::writer::MemoryReporter>(
+ nullptr, spill_threshold,
+ ::doris::snii::writer::MemoryReporter::CapPolicy::kSpillThreshold);
+ _term_buffer = std::make_unique<::doris::snii::writer::SpimiTermBuffer>(
+ _has_positions, spill_threshold, _memory_reporter.get());
+ // G09: join the PROCESS-WIDE build-RAM limiter. The per-writer spill
threshold above
+ // bounds one writer; a load keeps (tablets x concurrency) writers alive at
+ // once, none of which may ever reach it -- the global registry bounds
their
+ // SUM by asking the largest buffers to spill early (advisory flags honored
+ // on each writer's own thread; byte-identical output). Budget refreshed
+ // from the mutable config at every writer init; 0 disables (no
+ // registration, zero per-token overhead beyond the G08 path).
+ // G09 anti-storm knobs (see the config comments): the forced-spill floor
+ // gates both the owner-side honor (a request is a pending no-op until the
+ // reclaimable arena regrows past it) and the limiter's victim eligibility,
+ // and the run-file cap merge-compacts a writer's spill runs so the final
+ // k-way merge's fd fan-in stays bounded. Applied unconditionally -- the
+ // floor also protects test-seam requests, and the cap also bounds
+ // per-writer gate-2 runs when the global limiter is off.
+ _term_buffer->set_forced_spill_min_arena_bytes(
+
static_cast<uint64_t>(std::max<int64_t>(config::snii_forced_spill_min_arena_bytes,
0)));
+ _term_buffer->set_max_run_files(
+
static_cast<size_t>(std::max<int32_t>(config::snii_spill_max_run_files_per_buffer,
0)));
+ const int64_t global_budget =
config::snii_index_writer_global_memory_bytes;
+ if (global_budget > 0) {
Review Comment:
[P2] Apply a zero budget to the existing singleton. This mutable setting can
change from a positive value to zero, but the only `set_budget_bytes()` call is
inside this positive branch. Writers that registered earlier therefore keep
reporting against the stale positive budget and can still receive forced-spill
requests after the administrator disabled the limiter. Refresh the singleton
unconditionally, then decide whether the new writer should attach; add a
positive-to-zero wiring test with an older writer still live.
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java:
##########
@@ -1243,11 +1245,15 @@ public static TInvertedIndexFileStorageFormat
analyzeInvertedIndexFileStorageFor
return TInvertedIndexFileStorageFormat.V2;
} else if (invertedIndexFileStorageFormat.equalsIgnoreCase("v3")) {
return TInvertedIndexFileStorageFormat.V3;
Review Comment:
[P1] Gate SNII admission on backend capability. This explicit branch and the
default path return the new thrift value without checking target BEs. A
pre-change BE falls through that thrift enum to V3 while a current BE creates
SNII, so one tablet can acquire different physical formats during rolling
upgrade; an old proto2 reader also exposes unknown persisted enum 3 as the
field's V1 default. Please add a capability fence covering both wire and
persisted values, plus mixed-version creation/read tests.
##########
be/src/exprs/function/function_search.cpp:
##########
@@ -734,6 +787,93 @@ Status FunctionSearch::build_leaf_query(const
TSearchClause& clause,
*binding_key = binding.binding_key;
}
+ if (binding.use_snii_native_reader()) {
+ DORIS_CHECK(binding.inverted_reader != nullptr);
+ // The SNII reader answers a clause directly from a query type: it
tokenizes the value
+ // itself and owns the matching operator, so unlike the CLucene path
below there is no
+ // query tree to assemble here. RANGE and LIST reach the same TERM
fallback the CLucene
+ // path uses, because neither is implemented there either.
+ InvertedIndexQueryType snii_query_type = (clause_type == "RANGE" ||
clause_type == "LIST")
+ ?
InvertedIndexQueryType::EQUAL_QUERY
+ :
clause_type_to_query_type(clause_type);
+
+ if (clause_type == "TERM") {
+ // minimum_should_match ("at least N of M terms") has no SNII
query type: the reader
+ // only knows AND-all (MATCH_ALL_QUERY) or OR-all
(EQUAL_QUERY/MATCH_ANY_QUERY) of the
+ // terms it tokenizes internally, never a partial threshold. The
CLucene path builds
+ // an OccurBooleanQuery for this (function_search.cpp:913-926);
SNII cannot, so refuse
+ // explicitly instead of silently answering a plain OR query.
+ //
+ // But the CLucene path only ever reaches that OccurBooleanQuery
when the field is
+ // analysed (:925's `if (should_analyze)` short-circuits first): a
keyword (non-
Review Comment:
[P1] Defer the `minimum_should_match` rejection until after analysis. This
rejects every analyzed SNII TERM with a positive global threshold, whereas the
V3 path first tokenizes and accepts a one-token `TermQuery`; the threshold
matters only for a multi-token expansion. As a result, a single `body:alpha`
with `minimum_should_match:1` works on V3 but hard-fails on SNII, including
under boolean parents. Analyze first and reject only the unsupported
multi-token case (or implement it), with single-token parity coverage.
##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -0,0 +1,1182 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "storage/index/snii/snii_index_reader.h"
+
+#include <CLucene.h>
+#include <CLucene/util/stringUtil.h>
+#include <fmt/format.h>
+
+#include <algorithm>
+#include <atomic>
+#include <cctype>
+#include <charconv>
+#include <memory>
+#include <optional>
+#include <roaring/roaring.hh>
+#include <string>
+#include <string_view>
+#include <utility>
+
+#include "common/config.h"
+#include "runtime/exec_env.h"
+#include "runtime/query_context.h"
+#include "runtime/runtime_profile.h"
+#include "runtime/runtime_state.h"
+#include "storage/index/index_file_reader.h"
+#include "storage/index/index_reader_helper.h"
+#include "storage/index/inverted/analyzer/analyzer.h"
+#include "storage/index/inverted/analyzer/segment_analyzer_context.h"
+#include "storage/index/inverted/common/single_flight.h"
+#include "storage/index/inverted/inverted_index_cache.h"
+#include "storage/index/inverted/inverted_index_iterator.h"
+#include "storage/index/inverted/token_filter/common_grams_filter.h"
+#include "storage/index/snii/format/null_bitmap.h"
+#include "storage/index/snii/query/boolean_query.h"
+#include "storage/index/snii/query/count_query.h"
+#include "storage/index/snii/query/docid_sink.h"
+#include "storage/index/snii/query/internal/plain_term_routing.h"
+#include "storage/index/snii/query/phrase_query.h"
+#include "storage/index/snii/query/prefix_query.h"
+#include "storage/index/snii/query/regexp_query.h"
+#include "storage/index/snii/query/scoring_query.h"
+#include "storage/index/snii/query/term_query.h"
+#include "storage/index/snii/query/wildcard_query.h"
+#include "storage/index/snii/reader/logical_index_reader.h"
+#include "storage/index/snii/snii_doris_adapter.h"
+#include "storage/index/snii/snii_prx_profile.h"
+#include "storage/index/snii/stats/snii_stats_provider.h"
+#include "util/defer_op.h"
+#include "util/time.h"
+
+#ifdef BE_TEST
+namespace doris::snii::testing {
+namespace {
+
+std::atomic<uint64_t> prx_execution_profile_scope_constructions {0};
+std::atomic<uint64_t> prx_execution_profile_scope_flushes {0};
+
+} // namespace
+
+void record_prx_execution_profile_scope_construction() {
+ prx_execution_profile_scope_constructions.fetch_add(1,
std::memory_order_relaxed);
+}
+
+void record_prx_execution_profile_scope_flush() {
+ prx_execution_profile_scope_flushes.fetch_add(1,
std::memory_order_relaxed);
+}
+
+void reset_prx_execution_profile_scope_counters() {
+ prx_execution_profile_scope_constructions.store(0,
std::memory_order_relaxed);
+ prx_execution_profile_scope_flushes.store(0, std::memory_order_relaxed);
+}
+
+uint64_t prx_execution_profile_scope_construction_count() {
+ return
prx_execution_profile_scope_constructions.load(std::memory_order_relaxed);
+}
+
+uint64_t prx_execution_profile_scope_flush_count() {
+ return prx_execution_profile_scope_flushes.load(std::memory_order_relaxed);
+}
+
+} // namespace doris::snii::testing
+#endif
+
+namespace doris::segment_v2 {
+
+namespace {
+
+class RoaringDocIdSink final : public ::doris::snii::query::DocIdSink {
+public:
+ explicit RoaringDocIdSink(roaring::Roaring* bitmap) : _bitmap(bitmap) {
+ DCHECK(_bitmap != nullptr);
+ }
+
+ Status append_sorted(std::span<const uint32_t> docids) override {
+ if (!docids.empty()) {
+ _bitmap->addMany(docids.size(), docids.data());
+ }
+ return Status::OK();
+ }
+
+ Status append_range(uint32_t first, uint64_t last_exclusive) override {
+ if (last_exclusive > first) {
+ _bitmap->addRange(first, last_exclusive);
+ }
+ return Status::OK();
+ }
+
+ // Roaring addMany/addRange deduplicate and order natively, so multi-term
OR
+ // can stream each posting straight into the bitmap (no per-term vector +
merge).
+ bool dedups() const override { return true; }
+
+private:
+ roaring::Roaring* _bitmap;
+};
+
+struct SniiQueryExecutionResult {
+ std::shared_ptr<roaring::Roaring> bitmap;
+ std::vector<::doris::snii::query::PhraseMatch> phrase_matches;
+};
+
+std::vector<std::string> to_terms(const InvertedIndexQueryInfo& query_info) {
+ std::vector<std::string> terms;
+ terms.reserve(query_info.term_infos.size());
+ for (const auto& term_info : query_info.term_infos) {
+ DCHECK(term_info.is_single_term());
+ terms.push_back(term_info.get_single_term());
+ }
+ return terms;
+}
+
+bool uses_plain_term_frequency_scoring(InvertedIndexQueryType query_type,
+ const InvertedIndexQueryInfo&
query_info) {
+ return query_type == InvertedIndexQueryType::MATCH_ANY_QUERY ||
+ query_type == InvertedIndexQueryType::MATCH_ALL_QUERY ||
+ (query_type == InvertedIndexQueryType::MATCH_PHRASE_QUERY &&
+ query_info.term_infos.size() == 1);
+}
+
+bool uses_phrase_frequency_scoring(InvertedIndexQueryType query_type,
+ const InvertedIndexQueryInfo& query_info) {
+ return query_info.term_infos.size() > 1 &&
+ (query_type == InvertedIndexQueryType::MATCH_PHRASE_QUERY ||
+ query_type == InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY);
+}
+
+Status score_plain_term_candidates(const IndexQueryContextPtr& context,
+ std::string_view column_name,
+ const InvertedIndexQueryInfo& query_info,
+ const
::doris::snii::reader::LogicalIndexReader& logical_reader,
+ const
::doris::snii::stats::SniiStatsProvider& segment_stats,
+ const roaring::Roaring& final_candidates) {
+ DORIS_CHECK(context->collection_statistics != nullptr);
+ DORIS_CHECK(context->collection_similarity != nullptr);
+
+ const std::wstring field_name =
StringUtil::string_to_wstring(std::string(column_name));
+ const double collection_avgdl =
+
context->collection_statistics->get_or_calculate_avg_dl(field_name);
+ std::vector<::doris::snii::query::CollectionScoringTerm> scoring_terms;
+ scoring_terms.reserve(query_info.term_infos.size());
+ for (const auto& term_info : query_info.term_infos) {
+ DORIS_CHECK(term_info.is_single_term());
+ std::string physical_term;
+ bool representable = false;
+ RETURN_IF_ERROR(::doris::snii::query::internal::route_query_term(
+ logical_reader, term_info, &physical_term, &representable));
+ if (!representable) {
+ continue;
+ }
+ const std::string& logical_term = term_info.get_single_term();
+ const double idf =
context->collection_statistics->get_or_calculate_idf(
+ field_name, StringUtil::string_to_wstring(logical_term));
+ scoring_terms.push_back({.physical_term = std::move(physical_term),
.idf = idf});
+ }
+ DORIS_CHECK(final_candidates.isEmpty() || !scoring_terms.empty());
+
+ std::vector<::doris::snii::query::ScoredDoc> scored_docs;
+ RETURN_IF_ERROR(::doris::snii::query::scoring_query_candidates(
+ logical_reader, segment_stats, scoring_terms, final_candidates,
collection_avgdl,
+ ::doris::snii::query::Bm25Params {}, &scored_docs));
+ for (const auto& scored_doc : scored_docs) {
+ context->collection_similarity->collect(scored_doc.docid,
+
static_cast<float>(scored_doc.score));
+ }
+ return Status::OK();
+}
+
+Status score_phrase_matches(const IndexQueryContextPtr& context,
std::string_view column_name,
+ InvertedIndexQueryType query_type,
+ const InvertedIndexQueryInfo& query_info,
+ const ::doris::snii::reader::LogicalIndexReader&
logical_reader,
+ const ::doris::snii::stats::SniiStatsProvider&
segment_stats,
+ const roaring::Roaring& final_candidates,
+ const
std::vector<::doris::snii::query::PhraseMatch>& matches) {
+ DORIS_CHECK(context->collection_statistics != nullptr);
+ DORIS_CHECK(context->collection_similarity != nullptr);
+ DORIS_CHECK(uses_phrase_frequency_scoring(query_type, query_info));
+ DORIS_CHECK_EQ(final_candidates.cardinality(), matches.size());
+
+ const std::wstring field_name =
StringUtil::string_to_wstring(std::string(column_name));
+ const double collection_avgdl =
+
context->collection_statistics->get_or_calculate_avg_dl(field_name);
+ const size_t idf_term_count = query_type ==
InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY
+ ? query_info.term_infos.size() - 1
+ : query_info.term_infos.size();
+ double idf_sum = 0.0;
+ for (size_t i = 0; i < idf_term_count; ++i) {
+ const auto& term_info = query_info.term_infos[i];
+ DORIS_CHECK(term_info.is_single_term());
+ idf_sum += context->collection_statistics->get_or_calculate_idf(
+ field_name,
StringUtil::string_to_wstring(term_info.get_single_term()));
+ }
+
+ const auto scorer = ::doris::snii::query::ScorerContext::from_idf(idf_sum);
+ std::vector<::doris::snii::query::ScoredDoc> scored_docs;
+ scored_docs.reserve(matches.size());
+ for (const auto& match : matches) {
+ DCHECK(final_candidates.contains(match.docid));
+ DCHECK_NE(match.frequency, 0);
+ uint8_t norm = 0;
+ RETURN_IF_ERROR(segment_stats.encoded_norm(match.docid, &norm));
+ scored_docs.push_back({.docid = match.docid,
+ .score = scorer.score(match.frequency, norm,
collection_avgdl,
+
::doris::snii::query::Bm25Params {})});
+ }
+ for (const auto& scored_doc : scored_docs) {
+ context->collection_similarity->collect(scored_doc.docid,
+
static_cast<float>(scored_doc.score));
+ }
+ return Status::OK();
+}
+
+void parse_phrase_slop(std::string* query, InvertedIndexQueryInfo* query_info)
{
+ DCHECK(query != nullptr);
+ DCHECK(query_info != nullptr);
+ const auto is_digits = [](std::string_view str) {
+ return std::all_of(str.begin(), str.end(), [](unsigned char c) {
return std::isdigit(c); });
+ };
+
+ const size_t last_space_pos = query->find_last_of(' ');
+ if (last_space_pos == std::string::npos) {
+ return;
+ }
+ const size_t tilde_pos = last_space_pos + 1;
+ if (tilde_pos >= query->size() - 1 || (*query)[tilde_pos] != '~') {
+ return;
+ }
+
+ const size_t slop_pos = tilde_pos + 1;
+ std::string_view slop_str(query->data() + slop_pos, query->size() -
slop_pos);
+ if (slop_str.empty()) {
+ return;
+ }
+
+ bool ordered = false;
+ if (slop_str.size() == 1) {
+ if (!std::isdigit(static_cast<unsigned char>(slop_str[0]))) {
+ return;
+ }
+ } else if (slop_str.back() == '+') {
+ ordered = true;
+ slop_str.remove_suffix(1);
+ }
+
+ if (!is_digits(slop_str)) {
+ return;
+ }
+ auto result = std::from_chars(slop_str.begin(), slop_str.end(),
query_info->slop);
+ if (result.ec != std::errc()) {
+ return;
+ }
+ query_info->ordered = ordered;
+ *query = query->substr(0, last_space_pos);
+}
+
+std::shared_ptr<roaring::Roaring> docids_to_bitmap(const
std::vector<uint32_t>& docids) {
+ auto result = std::make_shared<roaring::Roaring>();
+ if (!docids.empty()) {
+ result->addMany(docids.size(), docids.data());
+ }
+ result->runOptimize();
+ return result;
+}
+
+// Runs `compute` under single-flight keyed by `key`: concurrent identical
queries collapse to a
+// single execution and the followers reuse the leader's bitmap.
`compute(out)` fills *out and
+// returns its Status; on overall success *result receives the bitmap. See
SingleFlight for why
+// this matters under a cold cache with parallel scanners hitting the same
segment.
+template <typename Compute>
+Status run_query_single_flight(
+ ::doris::segment_v2::inverted_index::SingleFlight<
+ std::pair<Status, std::shared_ptr<roaring::Roaring>>>& flight,
+ const std::string& key, std::shared_ptr<roaring::Roaring>* result,
+#ifdef BE_TEST
+ SniiIndexReader::SingleFlightFollowerJoinedObserver
follower_joined_observer,
+ void* follower_joined_opaque,
+ SniiIndexReader::SingleFlightLeaderBeforeComputeObserver
leader_before_compute_observer,
+ void* leader_before_compute_opaque,
+#endif
+ Compute&& compute) {
+ auto follower = flight.join_or_lead(key);
+ if (follower.has_value()) {
+#ifdef BE_TEST
+ if (follower_joined_observer != nullptr) {
+ follower_joined_observer(follower_joined_opaque);
+ }
+#endif
+ auto [leader_status, leader_bitmap] = follower->get();
+ if (leader_status.ok() && leader_bitmap != nullptr) {
+ *result = std::move(leader_bitmap);
+ return Status::OK();
+ }
+ // Leader failed; fall through and compute independently (rare error
path).
+ }
+ const bool is_leader = !follower.has_value();
+#ifdef BE_TEST
+ if (is_leader && leader_before_compute_observer != nullptr) {
+ leader_before_compute_observer(leader_before_compute_opaque);
+ }
+#endif
+
+ Status status = Status::OK();
+ std::shared_ptr<roaring::Roaring> bitmap;
+ {
+ // Publish to any waiting followers on every exit path (including
errors).
+ DEFER(if (is_leader) { flight.publish(key, std::make_pair(status,
bitmap)); });
+ status = compute(&bitmap);
+ }
+ RETURN_IF_ERROR(status);
+ *result = std::move(bitmap);
+ return Status::OK();
+}
+
+Status execute_snii_query(const ::doris::snii::reader::LogicalIndexReader&
logical_reader,
+ InvertedIndexQueryType query_type,
+ const InvertedIndexQueryInfo& query_info,
std::string_view search_str,
+ const std::vector<std::string>& terms, int32_t
max_expansions,
+ bool collect_phrase_frequency,
SniiQueryExecutionResult* result,
+ ::doris::snii::query::QueryProfile* profile) {
+ result->bitmap = std::make_shared<roaring::Roaring>();
+ result->phrase_matches.clear();
+ DORIS_CHECK(!collect_phrase_frequency ||
uses_phrase_frequency_scoring(query_type, query_info));
+ RoaringDocIdSink sink(result->bitmap.get());
+ std::vector<uint32_t> docids;
+ bool emitted_to_sink = false;
+ Status status;
+ switch (query_type) {
+ case InvertedIndexQueryType::EQUAL_QUERY:
+ case InvertedIndexQueryType::MATCH_ANY_QUERY:
+ status = terms.size() == 1
+ ? ::doris::snii::query::term_query(logical_reader,
terms.front(), &sink)
+ : ::doris::snii::query::boolean_or(logical_reader,
terms, &sink);
+ emitted_to_sink = true;
+ break;
+ case InvertedIndexQueryType::MATCH_ALL_QUERY:
+ if (terms.size() == 1) {
+ status = ::doris::snii::query::term_query(logical_reader,
terms.front(), &sink);
+ emitted_to_sink = true;
+ } else {
+ status = ::doris::snii::query::boolean_and(logical_reader, terms,
&docids);
+ }
+ break;
+ case InvertedIndexQueryType::MATCH_PHRASE_QUERY:
+ if (terms.size() == 1) {
+ status = ::doris::snii::query::term_query(logical_reader,
terms.front(), &sink);
+ emitted_to_sink = true;
+ } else {
+ status = collect_phrase_frequency
+ ?
::doris::snii::query::phrase_query_with_frequencies(
+ logical_reader, terms,
&result->phrase_matches, profile,
+ {.slop =
static_cast<uint32_t>(query_info.slop),
+ .ordered = query_info.ordered})
+ : ::doris::snii::query::phrase_query(
+ logical_reader, terms, &docids, profile,
+ {.slop =
static_cast<uint32_t>(query_info.slop),
+ .ordered = query_info.ordered});
+ }
+ break;
+ case InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY:
+ if (terms.size() == 1) {
+ status = ::doris::snii::query::prefix_query(logical_reader,
terms.front(), &sink,
+ max_expansions);
+ emitted_to_sink = true;
+ } else {
+ status = collect_phrase_frequency
+ ?
::doris::snii::query::phrase_prefix_query_with_frequencies(
+ logical_reader, terms,
&result->phrase_matches, profile,
+ max_expansions)
+ : ::doris::snii::query::phrase_prefix_query(
+ logical_reader, terms, &docids,
profile, max_expansions);
+ }
+ break;
+ case InvertedIndexQueryType::MATCH_REGEXP_QUERY:
+ status = ::doris::snii::query::regexp_query(logical_reader,
search_str, &sink,
+ max_expansions);
+ emitted_to_sink = true;
+ break;
+ case InvertedIndexQueryType::WILDCARD_QUERY:
+ status = ::doris::snii::query::wildcard_query(logical_reader,
search_str, &sink,
+ max_expansions);
+ emitted_to_sink = true;
+ break;
+ case InvertedIndexQueryType::LESS_THAN_QUERY:
+ case InvertedIndexQueryType::LESS_EQUAL_QUERY:
+ case InvertedIndexQueryType::GREATER_THAN_QUERY:
+ case InvertedIndexQueryType::GREATER_EQUAL_QUERY:
+ case InvertedIndexQueryType::RANGE_QUERY:
+ return Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>(
+ "SNII inverted index storage format does not support BKD/range
query");
+ default:
Review Comment:
[P1] Preserve `MATCH_PHRASE_EDGE` when the index uses SNII. The SQL function
maps to `MATCH_PHRASE_EDGE_QUERY`, but this switch has no case and returns
`INVERTED_INDEX_NOT_SUPPORTED`; neither top-level nor compound fallback treats
that status as row-evaluable. The same predicate works through V3 and already
has a row implementation, so moving to SNII turns it into a deterministic query
error despite fallback being enabled. Implement the native operator or return
an established fallback status, and test top-level and compound parity.
--
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]
