airborne12 commented on code in PR #69698:
URL: https://github.com/apache/doris/pull/69698#discussion_r4395692039
##########
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:
Fixed in 7bff7b8045. I did not collect the tablet set first and backfill;
the tablet set is now the `limit` smallest terms of the prefix over all
segments (`PrefixExpansionSet`), merged as segments are counted.
A term of that set has fewer than `limit` smaller terms in every segment
that holds it, so every such segment expanded to it and counted its documents,
whatever the order of the segments. In your example (limit 1, A = {rose}, B =
{red, rose}) the set is {red}, and `rose` is not scored, so its document
frequency of 1 is never used.
Covered by
`PrefixExpansionSetTest.KeepsTheSmallestTermsWhateverTheSegmentOrder` and a
two-segment case with `inverted_index_max_expansions = 1` in
`test_match_phrase_prefix_score`: the score equals idf(apple) + idf(red) =
1.1144 by hand, and 1.5844 with rose under a limit of 50.
##########
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:
Addressed in 7bff7b8045 by bounding what is kept, not by charging the
tracker. Please say if you want the accounting as well.
- `_prefix_expansions` keeps at most `limit` terms per prefix
(`PrefixExpansionSet`), and a session limit of 0 now scores the default 50
terms instead of the whole vocabulary: the sum of thousands of idfs ranks
nothing. So the per-scanner copies are bounded by the configured limit.
- `_term_doc_freqs` still holds the terms each segment selected, at most
`limit` per segment, so it can hold up to segments x limit entries. That is not
the dictionary, but it is not limit entries either.
- The standard containers are still not charged to the query MemTracker;
only `DorisVector` and `DorisMap` are tracked in this tree, and a tracked set
of strings would need its own allocator.
The query path expands the same prefix per segment with the same limit into
`std::vector<std::string>` and is equally untracked; with a limit of 0 it still
materializes every term there, which this change does not touch.
--
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]
