github-actions[bot] commented on code in PR #69190:
URL: https://github.com/apache/doris/pull/69190#discussion_r4294088825
##########
be/src/storage/index/inverted/inverted_index_parser.cpp:
##########
@@ -108,7 +108,11 @@ std::string
get_parser_phrase_support_string_from_properties(
it != properties.end()) {
return it->second;
}
- return INVERTED_INDEX_PARSER_PHRASE_SUPPORT_NO;
+ const auto analyzer = properties.find(INVERTED_INDEX_ANALYZER_NAME_KEY);
+ return get_parser_string_from_properties(properties) !=
INVERTED_INDEX_PARSER_NONE ||
+ (analyzer != properties.end() &&
!analyzer->second.empty())
Review Comment:
[P1] Preserve legacy docs-only analyzer metadata on upgrade
An omitted flag can already describe a persisted tokenized index with no
positions. In the `3.1.0-rc01` and `3.1.1-rc01` implementations, analyzer-only
DDL was accepted, but `Index` injected `support_phrase=true` only for a
`parser` property; the BE fallback was false and the writer set
`omitTermFreqAndPositions`. `Index.read()` preserves that omitted property
through Gson. After this change, the same unchanged metadata returns true, so
the reader advertises phrase/scoring support and multi-term `MATCH_PHRASE`
reaches `TermPositionsIterator` on files that never stored positions. New
writes also switch format under the identical property map. Preserve the old
effective false for persisted omissions, or migrate/rebuild those indexes
before enabling the new default; a reader-side inference cannot recover their
missing positions.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Index.java:
##########
@@ -77,10 +77,9 @@ public Index(long indexId, String indexName, List<String>
columns,
this.comment = comment;
if (indexType == IndexDef.IndexType.INVERTED) {
if (this.properties != null && !this.properties.isEmpty()) {
- if
(this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_PARSER_KEY)
- ||
this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_PARSER_KEY_ALIAS)
- ||
this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_ANALYZER_NAME_KEY)
- ||
this.properties.containsKey(InvertedIndexUtil.INVERTED_INDEX_NORMALIZER_NAME_KEY))
{
+ if
(!InvertedIndexUtil.INVERTED_INDEX_PARSER_NONE.equalsIgnoreCase(getInvertedIndexParser())
Review Comment:
[P2] Preserve phrase queries on normalizer-only indexes
For an index declared with `PROPERTIES("normalizer"="lowercase")`, this now
leaves `support_phrase` absent, so the BE fallback returns false. Such an index
still satisfies `InvertedIndexAnalyzer::should_analyzer()` and gets a FULLTEXT
reader; `InvertedIndexIterator::read_from_index()` then rejects `MATCH_PHRASE`,
`MATCH_PHRASE_PREFIX`, and `MATCH_PHRASE_EDGE`. For example, after inserting
`FOO BAR`, `WHERE content MATCH_PHRASE 'foo bar'` now raises `phrase queries
require setting support_phrase = true` instead of matching. Normalization
produces a single whole-value token, and `PhraseQuery::add()` already executes
that case as a term query without positions. Adapt the reader/phrase guard for
normalizer-only indexes alongside the docs-only default and cover this end to
end.
##########
be/src/storage/index/inverted/inverted_index_parser.cpp:
##########
@@ -108,7 +108,11 @@ std::string
get_parser_phrase_support_string_from_properties(
it != properties.end()) {
return it->second;
}
- return INVERTED_INDEX_PARSER_PHRASE_SUPPORT_NO;
+ const auto analyzer = properties.find(INVERTED_INDEX_ANALYZER_NAME_KEY);
+ return get_parser_string_from_properties(properties) !=
INVERTED_INDEX_PARSER_NONE ||
Review Comment:
[P2] Account for TOKENIZE's changed output default
This helper also controls the public JSON returned by `FunctionTokenize`:
`execute_impl()` passes the inferred flag to `_do_tokenize()`, which adds a
`position` member when it is true. Existing calls that omit `support_phrase`
now return different strings; for example, the Unicode call in
`regression-test/suites/inverted_index_p0/test_tokenize.groovy:100` has a
token-only expected result in
`regression-test/data/inverted_index_p0/test_tokenize.out:35`. The
Chinese/ICU/IK cases are affected as well, so their comparisons no longer
match. Preserve TOKENIZE's prior opt-in behavior at that consumer, or
explicitly adopt the JSON change and regenerate the affected goldens with
omitted/true/false coverage.
--
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]
