airborne12 commented on code in PR #66869:
URL: https://github.com/apache/doris/pull/66869#discussion_r3812818013
##########
be/src/storage/index/snii/snii_index_reader.cpp:
##########
@@ -656,11 +656,10 @@ Status SniiIndexReader::_query(const
IndexQueryContextPtr& context, const std::s
ctx->has_complete_common_grams_identity();
};
const bool safety_requires_plain = !common_grams_query_plan_enabled;
- // The raw cache key cannot prove whether the immutable segment analyzer
has CommonGrams until
- // its metadata is open. Delay every eligible forced-plain lookup, then
restore ordinary cache
- // access below only for a segment that cannot contain gram terms.
- const bool initial_force_plain = common_grams_query_eligible &&
safety_requires_plain;
- const bool initial_allow_result_cache = !actual_similarity &&
!initial_force_plain;
+ // An analyzed raw query can only share a cached result after the
immutable segment analyzer
+ // contract has been validated below. Patterns are analyzer-independent
and can still use the
+ // cache before opening the logical reader.
+ const bool initial_allow_result_cache = !actual_similarity &&
raw_pattern_query;
const bool defer_result_cache_lookup = !actual_similarity &&
!initial_allow_result_cache;
const InvertedIndexRawQuerySemantic raw_semantic {
Review Comment:
Confirmed and fixed in df75769.
Reproduced with a searcher-open counter: on a physical keyword-lane index
(`should_analyzer() == false`, so `STRING_TYPE`), a cold `EQUAL_QUERY` followed
by an identical warm one counted **2** segment opens with the searcher cache
disabled, where 1 is correct — the second ran `_get_logical_reader()` /
`open_snii_index()` before returning the already-warm bitmap.
Your reasoning about why that open teaches nothing there checks out:
`SniiIndexColumnWriter::init()` rejects a CommonGrams metadata seed with "SNII
CommonGrams metadata cannot be attached to a keyword analyzer" for exactly the
readers `ColumnReader` gives `STRING_TYPE`, and
`maybe_rebuild_segment_analyzer_context()` returns an empty optional as soon as
`segment_metadata == nullptr`.
Fix: the keyword lane now joins wildcard/regexp in the pre-open lookup,
keyed on `!InvertedIndexAnalyzer::should_analyzer(_index_meta.properties())` as
you suggested, rather than on the reader type it selects. Not keyed on
`PARSER_NONE`, per your caution — `get_parser_string_from_properties()` returns
`"none"` when the key is absent and `should_analyzer()` short-circuits to true
on a non-empty analyzer name, so a named analyzer would be misclassified; the
existing `CustomAnalyzerWithNoneParserRetainsAnalyzedTermCache` covers that
shape. Deferred admission for analyzer-backed readers is unchanged.
Test: `KeywordLaneWarmQueryCacheHitSkipsSegmentOpen` — a warm query-cache
case with the searcher cache disabled that asserts the open count stays at 1.
It fails with 2 opens before the fix and passes after.
--
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]
