github-actions[bot] commented on code in PR #69698:
URL: https://github.com/apache/doris/pull/69698#discussion_r4390657891
##########
be/src/storage/index/inverted/reader/collection_statistics.h:
##########
@@ -106,15 +107,27 @@ class CollectionStatistics {
const std::wstring& term);
MOCK_FUNCTION float get_or_calculate_avg_dl(const std::wstring&
lucene_col_name);
+ // The terms the collected `prefix` expanded to in any segment of the
tablet and that some
+ // returned row holds, in dictionary order; their idfs are in
get_or_calculate_idf. None for a
+ // prefix that was not collected, as for a SEARCH predicate.
+ MOCK_FUNCTION std::vector<std::wstring> get_prefix_expansions(
+ const std::wstring& lucene_col_name, const std::wstring& prefix);
+
// Replaces the collected counters with those of `other`. The derived
idf/avgdl caches are
// reset rather than copied, so the copy can be used by another thread
without sharing them.
void copy_collected_from(const CollectionStatistics& other);
private:
+ // The terms each collected prefix expanded to, by field and prefix.
+ using PrefixExpansions =
+ std::unordered_map<std::wstring,
+ std::unordered_map<std::wstring,
std::set<std::wstring>>>;
Review Comment:
[P2] Account for retained prefix-expansion memory
These standard containers retain dictionary-sized term sets without charging
the query MemTracker. For `inverted_index_max_expansions=0`, a broad two-token
phrase prefix can retain the entire matching tablet vocabulary in
`_prefix_expansions` and the expanded `_term_doc_freqs`;
`copy_collected_from()` then deep-copies both into every scanner. Unlike the
existing explicit accounting for SNII null-doc vectors, these allocations and
copies have no reservation or consume/release accounting, and the production
malloc hook forwards directly to jemalloc. A high-cardinality prefix therefore
bypasses query/workload memory limits and can drive process memory pressure.
Use tracked storage or complete explicit accounting, including term buffers and
scanner copies.
##########
be/src/storage/index/inverted/reader/collection_statistics.cpp:
##########
@@ -117,6 +119,35 @@ void add_term_doc_frequency(
(*logical_frequencies)[field][logical_term] += doc_frequency;
}
+Status segment_scoring_terms(index_query::IndexSource& source, const
CollectInfo& collect_info,
+ std::unordered_map<std::wstring,
std::set<std::wstring>>* expansions,
+ std::vector<std::string>* terms) {
+ DORIS_CHECK(expansions != nullptr);
+ DORIS_CHECK(terms != nullptr);
+ *terms = collect_info.unique_terms;
+ std::unordered_set<std::string> listed(terms->begin(), terms->end());
+ for (const auto& scoring_prefix : collect_info.scoring_prefixes) {
+ index_query::TermPattern pattern;
+
RETURN_IF_ERROR(index_query::TermPattern::create(index_query::TermPatternKind::kPrefix,
+
scoring_prefix.prefix, &pattern));
+ std::vector<std::string> expanded;
+ RETURN_IF_ERROR(source.expand_terms(
+ pattern,
+
index_query::expansion_limit(index_query::TermPatternKind::kPrefix,
+ scoring_prefix.max_expansions),
+ &expanded));
+ auto& prefix_terms =
(*expansions)[segment_v2::inverted_index::StringHelper::to_wstring(
+ scoring_prefix.prefix)];
+ for (auto& term : expanded) {
+
prefix_terms.insert(segment_v2::inverted_index::StringHelper::to_wstring(term));
+ if (listed.insert(term).second) {
+ terms->push_back(std::move(term));
Review Comment:
[P2] Count each tablet expansion in every collection segment
Only locally expanded terms enter this segment's document-frequency loop,
but `for_terms_and_prefix()` scores the union from all segments. With
`inverted_index_max_expansions=1`, let segment A contain `apple rose`, and B
contain `apple red` plus `apple rose`. A selects `rose`, B selects `red`; the
tablet union contains both, yet `df(rose)` becomes 1 instead of 2. For N=3 its
IDF is therefore 0.9808 instead of 0.4700, inflating every matching
phrase-prefix score. Both CLucene and SNII use this helper. Collect the tablet
expansion set first and count every selected term across every segment, or
backfill missing segment contributions; preserve hidden-row subtraction. The
single-segment regression does not cover this case.
--
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]
