airborne12 commented on code in PR #69790:
URL: https://github.com/apache/doris/pull/69790#discussion_r4419952144
##########
be/src/storage/index/inverted/reader/predicate_collector.cpp:
##########
@@ -285,6 +286,32 @@ void preserve_selected_index_metadata(const
ScoringIndexCandidates& candidates,
Review Comment:
Confirmed and fixed in 27cdbb570c, at the point where the failure happens
rather than in the collector.
Reproduced on 7881be50a9 with `INDEX idx_v(v) USING INVERTED` and `SELECT
score() ... WHERE CAST(v['n'] AS STRING) MATCH '1'`: V2/V3 fail with "No
segments* file found", SNII with "logical index is not a text inverted index".
The same query also failed before this PR when the VARIANT index is analyzed
(`parser=english`), so the collector is not the only way in; this PR made the
plain VARIANT index reach it too.
Keeping untyped VARIANT paths out of scoring in the collector would also
drop text paths, which score correctly today (`CAST(v['s'] AS STRING) MATCH
'abc'` returns BM25), and the collector cannot tell the two apart: the path
type is only known per segment. So statistics collection now treats a segment
whose index for the field is BKD like a segment without the path: it adds no
statistics (V2/V3: BKD data file in the compound directory, checked before
opening a CLucene reader; SNII: the logical index kind). Those rows still match
without the index and score 0; the text path keeps its score.
`test_match_keyword_score` adds a VARIANT case with a numeric path that is
BKD in one segment and text in another, plus a text path, on V2/V3/SNII; it
failed on all three before the change. 26 score-related suites pass.
##########
be/src/storage/index/inverted/reader/predicate_collector.cpp:
##########
@@ -285,6 +286,32 @@ void preserve_selected_index_metadata(const
ScoringIndexCandidates& candidates,
using namespace scoring_collection_detail;
+namespace {
+
+// The terms a MATCH scores, the ones its reader looks up. A keyword index
stores the whole value
+// as one term, case and spaces included: tag MATCH 'Hello World' scores the
single term
+// "Hello World", as SEARCH('tag:"Hello World"') does. Any other index cuts
the value with the
+// query's analyzer.
+Result<std::vector<TermInfo>> match_scoring_terms(const std::string& value,
+ InvertedIndexQueryType
query_type,
+ const TabletIndex&
index_meta,
+ const
InvertedIndexAnalyzerCtx& analyzer_ctx) {
+ if
(!inverted_index::InvertedIndexAnalyzer::should_analyzer(index_meta.properties()))
{
Review Comment:
Not reachable for a MATCH term, so no change here.
The collision needs a MATCH literal whose bytes are not valid UTF-8. The FE
only accepts a literal as the right operand, and none can carry such bytes.
Checked on 7881be50a9 against a keyword column holding `ra\xFF`, `ra\xFE` and
`ra`:
- `tag MATCH unhex('7261FF')` and `tag MATCH CAST(X'7261FF' AS STRING)` are
rejected: "Only support match left operand is SlotRef, right operand is
Literal".
- `tag MATCH X'7261FF'` is rejected: the operand is VARBINARY, not STRING.
- String literals come from the SQL text as UTF-8; `'ra\xFF'` is not an
escape for the byte and matches nothing.
- `tag MATCH 'ra'` matches only the `ra` row with its own idf (1.3862942,
same as `SEARCH('tag:ra')`).
The non-round-tripping terms that do exist come from the index dictionary,
i.e. prefix expansions, and `segment_scoring_terms` already skips those. Adding
the same check for direct terms would guard an input the FE cannot produce.
--
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]
