github-actions[bot] commented on code in PR #69052:
URL: https://github.com/apache/doris/pull/69052#discussion_r4292684784
##########
be/src/storage/index/inverted/query/engine/expand_query/expand_weight.cpp:
##########
@@ -85,12 +94,68 @@ std::shared_ptr<roaring::Roaring> ExpandWeight::_rows(const
QueryExecutionContex
ScorerPtr ExpandWeight::scorer(const QueryExecutionContext& context,
const std::string& binding_key) {
+ // The listing fast path discards term frequencies, so scored patterns
need term scorers.
+ if (_enable_scoring) {
+ return make_nullable_scorer(_scored_terms(context, binding_key),
+ logical_field_or_fallback(context,
binding_key, _field),
+ context.null_resolver);
+ }
auto scorer = std::make_shared<ConstScoreScorer<BitSetScorerPtr>>(
std::make_shared<BitSetScorer>(_rows(context, binding_key,
nullptr)));
return make_nullable_scorer(scorer, logical_field_or_fallback(context,
binding_key, _field),
context.null_resolver);
}
+ScorerPtr ExpandWeight::_scored_terms(const QueryExecutionContext& context,
+ const std::string& binding_key) {
+ index_query::TermPattern pattern;
+ THROW_IF_ERROR(index_query::TermPattern::create(_kind, _pattern,
&pattern));
+ auto source = lookup_source(_field, context, binding_key);
+ // An unbound field has no postings and contributes no matches.
+ if (source == nullptr) {
+ return std::make_shared<EmptyScorer>();
+ }
+ std::vector<std::string> terms;
+ THROW_IF_ERROR(source->expand_terms(
+ pattern, index_query::expansion_limit(_kind,
index_query::max_expansions(*_context)),
+ &terms));
+ // A valid pattern may match no dictionary terms.
+ if (terms.empty()) {
+ return std::make_shared<EmptyScorer>();
+ }
+ // Expansion terms are absent from collection statistics; use the bound
source's statistics.
+ index_query::IndexScoringStats stats;
+ THROW_IF_ERROR(source->scoring_stats(&stats));
+ DORIS_CHECK_GT(stats.doc_count, 0);
+ // Docs-only SNII still reports its physical token count. Only legacy
sources without
+ // that metadata report zero, so use unit average length only for those
sources.
+ const uint64_t token_count = stats.token_count != 0 ? stats.token_count :
stats.doc_count;
+ const float avgdl = static_cast<float>(token_count) / stats.doc_count;
+ DORIS_CHECK_GT(avgdl, 0.0F);
+ std::vector<ScorerPtr> scorers;
+ scorers.reserve(terms.size());
+ auto logical_field = logical_field_or_fallback(context, binding_key,
_field);
+ for (const auto& term : terms) {
+ uint64_t doc_freq = 0;
+ THROW_IF_ERROR(source->doc_freq(term, &doc_freq));
+ // The immutable source just enumerated this term and must have its
postings.
+ DCHECK_GT(doc_freq, 0);
+ const auto idf = static_cast<float>(std::log1p(
Review Comment:
[P2] Use collection statistics for expanded-term scores
These IDF and average-length values come from one physical segment, while
literal `TermQuery` scores use statistics collected across the tablet's
rowsets/segments. For an analyzed positional index, let segment A contain `foo
foo` and segment B contain `foo` plus 99 `bar` rows. A scored WILDCARD `f?o`
(or REGEXP `foo`) gives the two matching rows scores about 0.396 and 4.210
here. Compacting the unchanged rows into one segment changes those scores to
3.997 and 3.724, reversing the top result; the equivalent literal query already
uses the shared statistics. Please collect the expanded terms' statistics
across the same collection and build these similarities from that shared
context so segment layout does not determine relevance.
##########
be/src/storage/index/inverted/query/engine/expand_query/expand_weight.cpp:
##########
@@ -85,12 +94,68 @@ std::shared_ptr<roaring::Roaring> ExpandWeight::_rows(const
QueryExecutionContex
ScorerPtr ExpandWeight::scorer(const QueryExecutionContext& context,
const std::string& binding_key) {
+ // The listing fast path discards term frequencies, so scored patterns
need term scorers.
+ if (_enable_scoring) {
+ return make_nullable_scorer(_scored_terms(context, binding_key),
+ logical_field_or_fallback(context,
binding_key, _field),
+ context.null_resolver);
+ }
auto scorer = std::make_shared<ConstScoreScorer<BitSetScorerPtr>>(
std::make_shared<BitSetScorer>(_rows(context, binding_key,
nullptr)));
return make_nullable_scorer(scorer, logical_field_or_fallback(context,
binding_key, _field),
context.null_resolver);
}
+ScorerPtr ExpandWeight::_scored_terms(const QueryExecutionContext& context,
+ const std::string& binding_key) {
+ index_query::TermPattern pattern;
+ THROW_IF_ERROR(index_query::TermPattern::create(_kind, _pattern,
&pattern));
+ auto source = lookup_source(_field, context, binding_key);
+ // An unbound field has no postings and contributes no matches.
+ if (source == nullptr) {
+ return std::make_shared<EmptyScorer>();
+ }
+ std::vector<std::string> terms;
+ THROW_IF_ERROR(source->expand_terms(
+ pattern, index_query::expansion_limit(_kind,
index_query::max_expansions(*_context)),
+ &terms));
+ // A valid pattern may match no dictionary terms.
+ if (terms.empty()) {
+ return std::make_shared<EmptyScorer>();
+ }
+ // Expansion terms are absent from collection statistics; use the bound
source's statistics.
+ index_query::IndexScoringStats stats;
+ THROW_IF_ERROR(source->scoring_stats(&stats));
+ DORIS_CHECK_GT(stats.doc_count, 0);
+ // Docs-only SNII still reports its physical token count. Only legacy
sources without
+ // that metadata report zero, so use unit average length only for those
sources.
+ const uint64_t token_count = stats.token_count != 0 ? stats.token_count :
stats.doc_count;
+ const float avgdl = static_cast<float>(token_count) / stats.doc_count;
+ DORIS_CHECK_GT(avgdl, 0.0F);
+ std::vector<ScorerPtr> scorers;
+ scorers.reserve(terms.size());
+ auto logical_field = logical_field_or_fallback(context, binding_key,
_field);
+ for (const auto& term : terms) {
+ uint64_t doc_freq = 0;
+ THROW_IF_ERROR(source->doc_freq(term, &doc_freq));
+ // The immutable source just enumerated this term and must have its
postings.
+ DCHECK_GT(doc_freq, 0);
+ const auto idf = static_cast<float>(std::log1p(
+ (static_cast<double>(stats.doc_count) - doc_freq + 0.5) /
(doc_freq + 0.5)));
+ auto similarity = std::make_shared<BM25Similarity>(idf, avgdl);
+ auto postings = open_postings(*source, term, /*positions=*/false,
/*enable_scoring=*/true,
Review Comment:
[P2] Preserve batched reads for scored SNII expansions
On a cold SNII index with many non-inline matching terms, each
`open_postings` calls `open_term` with a private read wave, and constructing
`SegmentPostings` immediately reads that term's whole document/PRX span. The
loop finishes those reads one term at a time and retains every cursor and
fetched payload before constructing the union, even for `LIMIT 1`. Thus a
default 50-term expansion serializes 50 term reads, and a large or unlimited
`inverted_index_max_expansions` makes the postings working set grow with all
expanded terms. The existing `ListedTerms::scored_disjunction` /
`visit_term_postings` path already sums BM25 scores while batching and
releasing 32-term waves. Please reuse that path for batching sources instead of
keeping all independent cursors open.
--
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]
