github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4001779479
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java:
##########
@@ -80,6 +81,36 @@ private void readUnlock() {
lock.readLock().unlock();
}
+ // Legacy metadata may contain names that collide after locale-independent
normalization.
+ // Policy IDs are allocated monotonically, so the higher ID reproduces the
latest definition.
+ // Callers must hold the write lock.
+ private void registerPolicyNameLocked(IndexPolicy indexPolicy) {
+ String normalizedName = normalizeKey(indexPolicy.getName());
+ IndexPolicy current = nameToIndexPolicy.get(normalizedName);
+ if (current == null || indexPolicy.getId() > current.getId()) {
+ nameToIndexPolicy.put(normalizedName, indexPolicy);
Review Comment:
[P1] Propagate the collision winner to BE
This picks the higher-ID winner only in FE's name map. Both colliding
policies remain in `idToIndexPolicy`, so `getCopiedIndexPolicies()` and the
report path still push both IDs, while BE rejects whichever normalized name
arrives second. More importantly, an existing BE holding the lower-ID policy
rejects the FE winner on every report: FE continues resolving the higher-ID
tokenizer, BE continues resolving the lower-ID tokenizer, and reconciliation
retries forever because it compares IDs only. This is downstream of (and
distinct from) the FE-local replay/removal issue in thread 4001459133. Please
transmit/delete colliders according to the authoritative winner or apply the
same winner rule on BE, and cover an existing BE plus both arrival orders and
retry/drop behavior.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -43,16 +45,64 @@ public static String buildAnalyzerIdentity(
}
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
+ String builtinIkIdentity =
resolveBuiltinIkAnalyzerIdentity(properties, preferredAnalyzer);
+ if (builtinIkIdentity != null) {
+ return builtinIkIdentity;
+ }
// For custom analyzer/normalizer, resolve to underlying config to
build identity
return resolveAnalyzerIdentity(preferredAnalyzer,
defaultAnalyzerKey, log);
Review Comment:
[P1] Include outer filters in named-analyzer identities
The new legacy/custom collapse can equate runtime-distinct indexes here.
`{parser=ik}` now resolves to the smart synthetic identity, while
`{analyzer=smart,char_filter_type=char_replace,char_filter_pattern=a,char_filter_replacement=b}`
gets the same identity because the named-analyzer branch serializes only
policy `smart`'s `{tokenizer=ik_smart}` properties. FE accepts those outer
settings, and classic/SNII writers plus TOKENIZE/MATCH/search all wrap the
reader with `CharReplaceCharFilter` before custom IK runs (`aaaa` becomes
`bbbb`), so the streams differ. Existing threads guard overrides on literal
legacy/built-in IK, not this named-custom branch. Please incorporate
behavior-affecting outer properties or decline the cross-family collapse when
they are present, with CREATE and ALTER coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -230,11 +290,7 @@ private static String resolveCharFilterIdentity(String
filterList) {
sb.append(",");
}
- if (IndexPolicy.BUILTIN_CHAR_FILTERS.contains(filter)) {
- sb.append(filter);
- } else {
- sb.append(resolveComponentIdentity(filter,
IndexPolicyTypeEnum.CHAR_FILTER));
- }
+ sb.append(resolveComponentIdentity(filter,
IndexPolicyTypeEnum.CHAR_FILTER));
Review Comment:
[P1] Drop pass-through components from the analyzer identity
`empty` is a valid built-in character or token filter, but retaining it here
gives `{tokenizer=ik_smart}` and `{tokenizer=ik_smart,char_filter=empty}`
different identities (and the token-filter form has the same issue). BE's empty
filters delegate the reader/token stream unchanged, so these policies produce
the same terms, positions, offsets, lowercase behavior, and dictionary usage.
Both CREATE and ALTER can therefore accept duplicate inverted indexes on one
column despite the semantic-identity check. This is distinct from the existing
case/trim and legacy/custom spelling threads. Please canonicalize pass-through
components out of the identity and cover both duplicate-check paths.
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +88,70 @@ Token* IKTokenizer::next(Token* token) {
return nullptr;
}
- std::string& token_text = tokens_text_[buffer_index_++];
+ TokenData& token_data = tokens_[buffer_index_++];
// full-width to half-width, and lowercase
// TODO(ryan19929): do regularizeString in fillBuffer.
- CharacterUtil::regularizeString(token_text, this->lowercase);
- size_t size = std::min(token_text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
- token->setNoCopy(token_text.data(), 0, static_cast<int32_t>(size));
+ if (source_byte_offsets_enabled_) {
Review Comment:
[P1] Preserve offsets across pre-tokenizer character filters
This map starts from the reader that custom character filters have already
rewritten. A valid analyzer can compose an `icu_normalizer` character filter,
the new `ik_smart` tokenizer, and Pinyin with `ignore_pinyin_offset=false`;
default `nfkc_cf` changes original `ＬＩＵＤＥ` (15 UTF-8 bytes) to `liude` (5
bytes) before this code builds `[0,1,2,3,4,5]`. Pinyin then publishes `liu` at
`[0,3)` and `de` at `[3,5)` instead of the original spans `[0,9)` and `[9,15)`.
This is upstream of IK, unlike the existing IK-internal normalization and
post-tokenizer filter threads. Please carry character-filter offset correction
into this provenance map (or reject the unsupported composition) and cover the
actual ICU -> IK -> Pinyin chain, including reset/reuse.
--
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]
