github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4003502264
##########
be/src/storage/index/inverted/token_filter/pinyin_filter.cpp:
##########
@@ -234,7 +239,15 @@ bool PinyinFilter::processCurrentToken() {
// Convert to Unicode codepoints for processing
std::vector<UChar32> source_codepoints;
- convertToRunes(current_source_, source_codepoints);
+ current_runes_ = convertToRunes(current_source_, source_codepoints);
+
+ if (current_source_ == current_token_text_ &&
Review Comment:
[P1] Preserve the source displacement when trimming before offset
projection. With the existing default `trim_whitespace=true` and
`ignore_pinyin_offset=false`, a valid `keyword -> pinyin` chain over `" 刘德华
"` receives the upstream span `[0,13)`, then trims two leading bytes here and
rebuilds rune positions from `刘德华` starting at zero. Because the strings
differ, this condition also skips any upstream boundary map, and
`setTokenAttributes()` rebases partial terms on the unchanged start:
`liu`/`de`/`hua` become `[0,3)`/`[3,6)`/`[6,9)` instead of
`[2,5)`/`[5,8)`/`[8,11)`. The existing whitespace test uses this exact input
but asserts only terms. Please retain the leading/trailing trim boundaries,
slice/compose the source map through them, and add exact offset assertions
including reset/reuse.
##########
be/src/storage/index/inverted/tokenizer/keyword/keyword_tokenizer.h:
##########
@@ -45,6 +45,8 @@ class KeywordTokenizer : public DorisTokenizer {
int32_t length = std::min(_char_length, MAX_TOKEN_LENGTH_LIMIT);
std::string_view term(_char_buffer, length);
set(token, term);
+ token->setStartOffset(0);
+ token->setEndOffset(length);
Review Comment:
[P1] Preserve source offsets through character filters here too. This
tokenizer now publishes `[0, length)`, but `length` is measured from the reader
after custom character filters. For a valid `icu_normalizer -> keyword ->
pinyin` analyzer with `ignore_pinyin_offset=false`, full-width `ＬＩＵＤＥ` becomes
the five-byte `liude`: whole-token alternatives receive `[0,5)` instead of
`[0,15)`, while split `liu`/`de` terms use normalized `[0,3)`/`[3,5)` instead
of `[0,9)`/`[9,15)`. `StandardTokenizer` has the same separate path because its
new scanner offsets are also post-filter positions. Only `IKTokenizer`
currently applies `DorisCharFilter::correct_offset()` and exposes corrected
per-rune provenance. This is distinct from the existing IK correction thread
and the direct Keyword/Standard offset thread because neither covers a
pre-tokenizer character filter. Please correct the endpoints and partial-token
source-boundary provenance for both tokenizers, with ICU -> Keyword/Standar
d -> offset-aware Pinyin reset tests.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java:
##########
@@ -80,11 +81,44 @@ 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);
+ }
+ if (current != null && current.getId() != indexPolicy.getId()) {
+ LOG.warn("Index policies '{}' (id={}) and '{}' (id={}) have the
same normalized name; "
+ + "using the policy with the higher ID for name
lookup",
+ current.getName(), current.getId(), indexPolicy.getName(),
indexPolicy.getId());
+ }
+ }
+
+ private void unregisterPolicyNameLocked(IndexPolicy indexPolicy) {
+ String normalizedName = normalizeKey(indexPolicy.getName());
+ IndexPolicy current = nameToIndexPolicy.get(normalizedName);
+ if (current == null || current.getId() != indexPolicy.getId()) {
+ return;
+ }
+ nameToIndexPolicy.remove(normalizedName);
+ for (IndexPolicy remaining : idToIndexPolicy.values()) {
+ if (normalizedName.equals(normalizeKey(remaining.getName()))) {
+ registerPolicyNameLocked(remaining);
+ }
+ }
+ }
+
public List<IndexPolicy> getCopiedIndexPolicies() {
List<IndexPolicy> copiedPolicies = Lists.newArrayList();
readLock();
try {
- copiedPolicies.addAll(idToIndexPolicy.values());
+ // Only transmit the authoritative policy for each normalized
name. Legacy images may
+ // contain collisions, but sending both definitions would make BE
choose based on
+ // arrival order and repeatedly diverge from FE during
reconciliation.
+ copiedPolicies.addAll(nameToIndexPolicy.values());
Review Comment:
[P1] Do not silently rebind existing analyzers when collapsing legacy
locale-dependent names. On an old Turkish-default FE, `IK_SMART` and `ik_smart`
have distinct keys and can be different tokenizer policies; an analyzer
persisted with `tokenizer=IK_SMART` therefore legitimately binds the first one.
If the later `ik_smart` policy has the higher ID, replay now selects it and
this method sends only that winner, causing BE reconciliation to delete the
original. The unchanged analyzer reference is then normalized and resolved to
the other policy by both FE identity construction and BE analyzer construction.
For example, keyword versus standard definitions change existing index/query
behavior from one token to two. This is downstream of the existing
reachability/convergence threads: selecting and propagating a stable winner
converges, but does not preserve persisted dependent semantics. Please preserve
legacy exact-name bindings or reject/mark referenced collisions for explicit
migrat
ion, and test journal/image plus BE reconciliation with a dependent analyzer
and emitted terms before and after upgrade.
--
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]
