github-actions[bot] commented on code in PR #69791:
URL: https://github.com/apache/doris/pull/69791#discussion_r4408019708
##########
be/src/storage/predicate_collector.cpp:
##########
@@ -300,7 +296,12 @@ Status MatchPredicateCollector::collect(RuntimeState*
state, const TabletSchemaS
options.timezone = &state->timezone_obj();
const std::string value = right_literal->value(options);
std::vector<TermInfo> term_infos;
- if (match_analyzer_ctx != nullptr) {
+ if (!InvertedIndexAnalyzer::should_analyzer(index_meta->properties()))
{
+ // A keyword index stores the whole value as one term, case and
spaces included, and
+ // its reader looks it up unanalyzed: tag MATCH 'Hello World'
scores the single term
+ // "Hello World", as SEARCH('tag:"Hello World"') does.
+ term_infos.emplace_back(value);
Review Comment:
[P2] Keep BM25 statistics separate for each selected index
On a STRING column with both a `parser=standard, support_phrase=true` index
and a keyword index, insert `apple banana`, `apple`, and `pear`, then score
`(body MATCH 'apple' USING ANALYZER standard) OR (body MATCH 'apple' USING
ANALYZER none)`. The clauses need different `df(apple)` values (2 versus 1) and
average lengths (4/3 versus 1), but this newly admitted keyword term merges
into the same field-only `CollectInfoMap` entry. Only the first `index_meta`
survives, and `CollectionStatistics`/`BM25Similarity` use field/term keys
without index identity, so at least one clause receives incorrect statistics
regardless of collection order. Before this change the keyword clause was
skipped and did not contaminate analyzed scoring. Carry selected-index identity
through collection and scorer lookups, and cover this same-column mixed-index
query.
##########
be/src/storage/index/index_reader_helper.h:
##########
@@ -106,33 +107,34 @@ class IndexReaderHelper {
static bool is_need_similarity_score(InvertedIndexQueryType query_type,
const TabletIndex* index_meta) {
- if (query_type == InvertedIndexQueryType::MATCH_ANY_QUERY ||
- query_type == InvertedIndexQueryType::MATCH_ALL_QUERY ||
- query_type == InvertedIndexQueryType::MATCH_PHRASE_QUERY ||
- query_type == InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY) {
- const auto& properties = index_meta->properties();
- if (get_parser_phrase_support_string_from_properties(properties) ==
- INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES ||
- is_normalizer_only(properties)) {
- return true;
- }
- }
- return false;
+ return (query_type == InvertedIndexQueryType::MATCH_ANY_QUERY ||
+ query_type == InvertedIndexQueryType::MATCH_ALL_QUERY ||
+ query_type == InvertedIndexQueryType::MATCH_PHRASE_QUERY ||
+ query_type ==
InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY) &&
+ can_score_match(index_meta);
}
static bool is_need_similarity_score(TExprOpcode::type query_type,
const TabletIndex* index_meta) {
- if (query_type == TExprOpcode::MATCH_ANY || query_type ==
TExprOpcode::MATCH_ALL ||
- query_type == TExprOpcode::MATCH_PHRASE ||
- query_type == TExprOpcode::MATCH_PHRASE_PREFIX) {
- const auto& properties = index_meta->properties();
- if (get_parser_phrase_support_string_from_properties(properties) ==
- INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES ||
- is_normalizer_only(properties)) {
- return true;
- }
+ return (query_type == TExprOpcode::MATCH_ANY || query_type ==
TExprOpcode::MATCH_ALL ||
+ query_type == TExprOpcode::MATCH_PHRASE ||
+ query_type == TExprOpcode::MATCH_PHRASE_PREFIX) &&
+ can_score_match(index_meta);
+ }
+
+private:
+ static bool can_score_match(const TabletIndex* index_meta) {
+ const auto& properties = index_meta->properties();
+ // A keyword index keeps each value as one term, without positions or
norms, and its
+ // postings score with unit frequency and length whatever
support_phrase says, as an
+ // Elasticsearch keyword field does: over tags (x, x, y, z), tag MATCH
'x' scores
+ // idf(x) = ln(2) for both matching rows, the same as SEARCH('tag:x').
+ if
(!inverted_index::InvertedIndexAnalyzer::should_analyzer(properties)) {
Review Comment:
[P2] Restrict no-analyzer scoring eligibility to string indexes
Numeric BKD indexes also have no analyzer, so this condition admits them
into the text scoring collector. For a nonempty duplicate-key table with `n
INT` and an inverted index on `n`, `SELECT n, score() FROM t WHERE CAST(n AS
STRING) MATCH '1'` is accepted and enters common-expression pushdown when
`enable_common_expr_pushdown=true`. The collector strips the cast and selects
the original numeric index, then `CollectionStatistics` tries to open its BKD
directory as a CLucene text index (or extracts `FulltextIndexSearcherPtr` from
a cached BKD reader). This fails before the normal incompatible-cast fallback;
previously the no-analyzer metadata was skipped. Check the actual column/reader
type before admitting these indexes and add this casted-numeric regression.
--
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]
