airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4075514754
##########
be/src/storage/index/inverted/tokenizer/pinyin/pinyin_tokenizer.cpp:
##########
@@ -206,14 +220,15 @@ Token* PinyinTokenizer::next(Token* token) {
size_t size = std::min(text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
token->setNoCopy(text.data(), 0, static_cast<int32_t>(size));
+ int32_t start = item.start_offset;
+ int32_t end = item.end_offset;
if (config_->ignorePinyinOffset) {
- int total_byte_length = runes_.empty() ? 0 :
runes_.back().byte_end;
- token->setStartOffset(0);
- token->setEndOffset(total_byte_length);
- } else {
- token->setStartOffset(item.start_offset);
- token->setEndOffset(item.end_offset);
+ start = 0;
+ end = runes_.empty() ? 0 : runes_.back().byte_end;
}
+ token->setStartOffset(correct_source_start_offset(start));
+ token->setEndOffset(correct_source_offset(end));
+ publishCandidateProvenance(std::string_view(text.data(), size), start,
end);
Review Comment:
Confirmed and fixed in 5843206b21c.
I reproduced it first: the new test asserts the clipped prefix, and against
the unfixed tokenizer it failed exactly as described - for a 300-byte over-cap
original candidate `token.endOffset()` was 300 (the emitted prefix claimed the
whole input) and `get_source_byte_offsets()` came back empty, i.e. the
conservative whole-input span, instead of 256 exact entries.
The fix clips on a rune boundary through a shared `utf8_prefix_at_most()`
helper, lifted out of `IKTokenizer.cpp` into `token_stream.h` so both
tokenizers use one implementation, and narrows the end offset and the published
provenance to the emitted prefix when the candidate is the untouched source
slice.
Coverage in
`PinyinFilterTest.TestPinyinTokenizerClipsOriginalCandidateProvenance`: 300
ASCII bytes give a 255-byte term with `endOffset` 255 and 256 source offsets
ending at 255; 200 x U+00E9 give the split-rune case, where the 255-byte cap
would cut the 128th 2-byte rune, so the term is clipped to 254 bytes with
`endOffset` 254 and 128 source offsets.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java:
##########
@@ -55,13 +56,64 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {
private final Map<Long, IndexPolicy> idToIndexPolicy = Maps.newHashMap();
// Keys are normalized to lowercase for case-insensitive lookup
private final Map<String, IndexPolicy> nameToIndexPolicy =
Maps.newHashMap();
+ // Legacy metadata can contain case-distinct names that share a normalized
key. Keep exact
+ // bindings separately so a saved analyzer continues to resolve its
original component.
+ private final transient Map<String, IndexPolicy> exactNameToIndexPolicy =
Maps.newHashMap();
/**
* Normalize policy name to lowercase for case-insensitive lookup.
* Policy names are case-insensitive in Doris.
*/
private static String normalizeKey(String name) {
- return name == null ? null : name.trim().toLowerCase();
+ return name == null ? null : name.trim().toLowerCase(Locale.ROOT);
+ }
+
+ private static String exactKey(String name) {
+ return name == null ? null : name.trim();
+ }
+
+ // Callers hold either the read or write lock. Prefer an exact legacy name
binding and
+ // retain normalized lookup only for interactive case-insensitive fallback.
+ private IndexPolicy getPolicyByNameLocked(String name) {
+ IndexPolicy exactPolicy = exactNameToIndexPolicy.get(exactKey(name));
+ return exactPolicy != null ? exactPolicy :
nameToIndexPolicy.get(normalizeKey(name));
+ }
+
+ // Callers hold either the read or write lock. BE dispatches a canonical
built-in analyzer, then an
+ // exact policy, then a built-in by normalized name; return that built-in,
or null for a policy.
+ private String resolveTopLevelBuiltinLocked(String name, Set<String>
builtins) {
+ String exactName = exactKey(name);
+ if (IndexPolicy.BUILTIN_ANALYZERS.contains(exactName) &&
builtins.contains(exactName)) {
+ return exactName;
+ }
+ if (exactNameToIndexPolicy.containsKey(exactName)) {
+ return null;
+ }
+ String normalizedName = normalizeKey(name);
+ return builtins.contains(normalizedName) ? normalizedName : null;
Review Comment:
Confirmed, and it was a regression introduced by the earlier
canonicalization fix. Fixed in 79715d0b1b2.
`find_top_level_policy_locked()` resolves a top-level name as exact policy
(trimmed, case sensitive) ->
`BUILTIN_NORMALIZERS.contains(normalize_name(name))` -> normalized policy
lookup. With an exact policy named `lowercase` present, persisting the
canonical `lowercase` therefore binds that policy, while the user's `LowerCase`
still reaches the built-in normalizer.
`resolveTopLevelBuiltinLocked()` now returns the spelling that actually
reaches the built-in: the canonical name normally, but the given spelling when
an exact policy occupies the canonical one. This is deliberately limited to
built-in normalizers, because `InvertedIndexAnalyzer::is_builtin_analyzer()` is
consulted before the policy manager, so a canonical built-in analyzer name can
never be shadowed by a policy - the test asserts `Standard` still persists as
`standard` even with an exact `standard` policy replayed.
Tests:
`InvertedIndexPropertiesTest#testExactLowercasePolicyKeepsMixedCaseBuiltinNormalizerBinding`
for CREATE and MATCH binding, and
`SchemaChangeHandlerTest#testMixedCaseNormalizerKeepsBuiltinBindingWhenExactPolicyShadowsIt`
for ALTER plus CREATE with both spellings on one column.
--
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]
