airborne12 commented on code in PR #69698:
URL: https://github.com/apache/doris/pull/69698#discussion_r4396134786
##########
be/src/storage/index/inverted/reader/collection_statistics.cpp:
##########
@@ -117,6 +119,56 @@ void add_term_doc_frequency(
(*logical_frequencies)[field][logical_term] += doc_frequency;
}
+Status segment_scoring_terms(index_query::IndexSource& source, const
CollectInfo& collect_info,
+ const VisibleDocFreq& visible_doc_freq,
+ FieldPrefixExpansions* 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));
+ const auto wanted = static_cast<size_t>(scoring_prefix.max_expansions);
+ std::vector<std::string> visible;
+ for (int32_t limit = scoring_prefix.max_expansions;; limit *= 2) {
+ std::vector<std::string> expanded;
+ RETURN_IF_ERROR(source.expand_terms(pattern, limit, &expanded));
+ visible.clear();
+ for (auto& term : expanded) {
+ uint64_t doc_freq = 0;
+ RETURN_IF_ERROR(visible_doc_freq(term, &doc_freq));
+ if (doc_freq > 0) {
+ visible.push_back(std::move(term));
+ }
+ }
+ // The dictionary ran out of terms, or the first `wanted` visible
ones are found.
+ if (expanded.size() < static_cast<size_t>(limit) || visible.size()
>= wanted) {
+ break;
+ }
+ }
+ // A read with doubled limit can find more visible terms than the
prefix keeps; the
+ // dictionary order puts the ones to keep first.
+ if (visible.size() > wanted) {
+ visible.resize(wanted);
+ }
+ auto& prefix_terms =
+ expansions
+
->try_emplace(segment_v2::inverted_index::StringHelper::to_wstring(
+ scoring_prefix.prefix),
+ scoring_prefix.max_expansions)
+ .first->second;
+ for (auto& term : visible) {
+
prefix_terms.insert(segment_v2::inverted_index::StringHelper::to_wstring(term));
Review Comment:
Fixed in 59f3dc7db3, differently from the "preserve raw term identity" you
suggest. The statistics, the idf lookups and `for_terms` are all keyed by wide
string, and carrying raw bytes through them would touch every consumer,
including the analyzed query terms that are always valid. Instead, an expanded
term whose wide-string conversion does not convert back to its bytes has no key
of its own and is not scored. It takes none of the `max_expansions` places
either, so the next valid term is used, as for a term that only hidden rows
hold.
In your example the three `r...a` terms are therefore skipped, `df(ra)` is
never accumulated, and the score stays `idf(apple)` alone. The cost is that a
malformed term contributes nothing to the idf, which is the same as before this
PR.
`phrase_prefix_query_test` covers it with a dictionary of "ra\xFF",
"ra\xFE", rb, rc and rd under a limit of 2: it expands to rb and rc. Without
the check the same test fails with the single key `ra`. I did not add an SNII
end-to-end case with malformed bytes; the helper is shared by CLucene and SNII
and the test uses a dictionary-only source.
--
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]
