[fix](index) Drop support_phrase from indexes that do not tokenize - #67219
Open
airborne12 wants to merge 1 commit into
Open
[fix](index) Drop support_phrase from indexes that do not tokenize#67219airborne12 wants to merge 1 commit into
airborne12 wants to merge 1 commit into
Conversation
airborne12
requested review from
csun5285,
eldenmoon,
gavinchou and
yiguolei
as code owners
August 27, 2026 12:02
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Problem Summary: `support_phrase=true` asks the BE to persist term positions, and a position is only observable when a query can supply a second term to match it against. `InvertedIndexAnalyzer::get_analyse_result()` short-circuits on `!should_analyzer()` and returns the entire search string as ONE term, for every analysis purpose. Every phrase variant -- MATCH_PHRASE, MATCH_PHRASE_PREFIX, MATCH_PHRASE_EDGE -- sources its terms from there, so a phrase against an index with no analyzer is always a single-term phrase, which is a term query. The option describes something no query can reach. FE was manufacturing exactly that combination. `Index`'s constructor unconditionally defaulted `support_phrase` to `true` for every inverted index, including `parser=none` ones, so a keyword index reached the BE carrying an option it can never honour. Downstream, `IndexReaderHelper::is_need_similarity_score()` read that option alone and reported such an index as scoreable, letting relevance work be planned against positions and norms that were never written. The fix aligns both sides on the same predicate: - FE keeps `support_phrase` only where the BE would run an analyzer, and removes the key otherwise, so new tablet metadata no longer carries it. - BE gains `IndexReaderHelper::persists_scoring_inputs()`, which requires both `support_phrase=yes` and `InvertedIndexAnalyzer::should_analyzer()`. Both `is_need_similarity_score()` overloads route through it. Tablet metadata already written to disk still holds the stale option, so the FE-side change alone is not sufficient and the guard has to be repeated on the read path. `should_analyzer()` is the exact line, and it is subtler than "has a parser": `get_analyzer_name_from_properties()` falls back to the **normalizer** key, so a `normalizer`-only index IS analyzed on the BE and is served by a FULLTEXT reader. `match.cpp` rejects `MATCH_PHRASE` outright on a FULLTEXT-served index whose `support_phrase` is absent, so dropping the key there would turn a working (degenerate, single-term) phrase query into a hard error. `isTokenizedInvertedIndex` is therefore composed from `InvertedIndexProperties.getPreferredAnalyzer()` and `getInvertedIndexParser()` -- the FE mirrors of the two resolvers `should_analyzer` itself uses -- rather than re-reading the property keys by hand, so the two sides cannot drift apart. `testSupportPhraseKeptForNormalizerIndexes` and the normalizer case in `IsNeedSimilarityScoreRequiresATokenizer` pin both halves. ARRAY deserves an explicit note, because the obvious argument does not hold for it. `InvertedIndexUtil::checkInvertedIndexParser` forces every ARRAY inverted index to `parser=none`, yet an ARRAY row emits one term per element and `SniiIndexColumnWriter::_add_array_values` does advance positions between them (CLucene writes the same shape). Those positions are nevertheless unreachable, for the query-side reason above -- not because the document holds a single term. `UntokenizedQueriesCannotObserveAPosition` pins that invariant so a future analyzer change cannot silently invalidate it. `test_index_meta.groovy` asserted the exact `SHOW INDEX` property string for a `parser=none` index; `support_phrase` is no longer part of it. The `parser=standard` assertions in the same file are unchanged, which is the intended split. For reference, Elasticsearch rejects the equivalent shape at mapping time: a `keyword` field's `index_options` accepts only `docs` and `freqs`, never `positions`. Release note: None
airborne12
force-pushed
the
snii-phrase-normalize
branch
from
August 27, 2026 14:09
1826997 to
d9576ce
Compare
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16909 ms |
Contributor
TPC-DS: Total hot run time: 81942 ms |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
ClickBench: Total hot run time: 14.49 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
support_phrase=trueasks the BE to persist term positions, and a position is onlyobservable when a query can supply a second term to match it against.
InvertedIndexAnalyzer::get_analyse_result()short-circuits on!should_analyzer()andreturns the entire search string as ONE term, for every analysis purpose. Every phrase
variant —
MATCH_PHRASE,MATCH_PHRASE_PREFIX,MATCH_PHRASE_EDGE— sources its termsfrom there, so a phrase against an index with no tokenizer is always a single-term phrase,
which is a term query. The option describes something no query can reach.
FE was manufacturing exactly that combination.
Index's constructor unconditionallydefaulted
support_phrasetotruefor every inverted index, includingnormalizer-onlyindexes and
parser=noneones, so a keyword index reached the BE carrying an option it cannever honour. Downstream,
IndexReaderHelper::is_need_similarity_score()read that optionalone and reported such an index as scoreable, letting relevance work be planned against
positions and norms that were never written.
The fix aligns both sides on the same predicate:
support_phraseonly when the index actually tokenizes — an explicitanalyzer, or aparserother thannone— and removes the key otherwise, so newtablet metadata no longer carries it.
IndexReaderHelper::persists_scoring_inputs(), which requires bothsupport_phrase=yesandInvertedIndexAnalyzer::should_analyzer(). Bothis_need_similarity_score()overloads route through it. Tablet metadata already writtento disk still holds the stale option, so the FE-side change alone is not sufficient and
the guard has to be repeated on the read path.
ARRAY deserves an explicit note, because the obvious argument does not hold for it.
InvertedIndexUtil::checkInvertedIndexParserforces every ARRAY inverted index toparser=none, yet an ARRAY row emits one term per element andSniiIndexColumnWriter::_add_array_valuesdoes advance positions between them (CLucenewrites the same shape). Those positions are nevertheless unreachable, for the query-side
reason above — not because the document holds a single term.
UntokenizedQueriesCannotObserveAPositionpins that invariant so a future analyzer changecannot silently invalidate it.
For reference, Elasticsearch rejects the equivalent shape at mapping time: a
keywordfield's
index_optionsaccepts onlydocsandfreqs, neverpositions.Release note
None
Check List (For Author)
Test
IndexTest.testSupportPhraseKeptForTokenizingIndexes,IndexTest.testSupportPhraseDroppedForNonTokenizingIndexes(covers thenormalizer-only,
parser=noneand keyword shapes)IndexReaderHelperTest.IsNeedSimilarityScoreRequiresATokenizer,IndexReaderHelperTest.UntokenizedQueriesCannotObserveAPositionIndex.javafails the FE normalizerassertion; reverting
index_reader_helper.hmakes the keyword andparser=noneshapes report
true.classes touching
Index/support_phrasegreen (149 tests).Behavior changed:
support_phrase, andis no longer reported as carrying scoring inputs. No query result changes: such an
index can only ever be queried with a single-term phrase, which is a term query.
Existing tablets keep their stored property; the BE-side guard handles them.
Does this need documentation?
Check List (For Reviewer who merge this PR)