github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4046325317
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/InvertedIndexUtil.java:
##########
@@ -423,17 +445,18 @@ public static boolean isAnalyzerMatched(Map<String,
String> properties, String a
buildAnalyzerIdentity(properties));
}
+ String resolvedAnalyzer = resolveAnalyzerName(normalizedAnalyzer);
String preferredAnalyzer =
InvertedIndexProperties.getPreferredAnalyzer(properties);
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
- return normalizedAnalyzer.equalsIgnoreCase(preferredAnalyzer);
+ return
resolvedAnalyzer.equals(resolveAnalyzerName(preferredAnalyzer));
}
String parser =
InvertedIndexProperties.getInvertedIndexParser(properties);
if (Strings.isNullOrEmpty(parser)) {
- return normalizedAnalyzer.equalsIgnoreCase("default")
- ||
normalizedAnalyzer.equalsIgnoreCase(INVERTED_INDEX_PARSER_NONE);
+ return resolvedAnalyzer.equals("default")
+ || resolvedAnalyzer.equals(INVERTED_INDEX_PARSER_NONE);
}
- return normalizedAnalyzer.equalsIgnoreCase(parser);
+ return resolvedAnalyzer.equals(parser.trim().toLowerCase(Locale.ROOT));
Review Comment:
[P1] Do not admit IK configurations that MATCH cannot disambiguate
This now lets smart/max-word and lowercase variants coexist on one column,
but `USING ANALYZER ik` matches every `parser=ik` index here; FE picks the
first, every BE reader key is also `ik`, and BE picks the lowest ID. In the new
four-index regression, an explicit built-in `ik` request (whose identity is
max-word) can therefore use the first smart/lowercase-disabled index instead.
`清华大学 MATCH 清华` distinguishes this: max-word indexes `清华`, smart does not,
while the added `abc` assertion cannot detect the rebinding. This is distinct
from the existing case-spelling collision because these configurations
intentionally share the canonical name. Carry semantic identity through
FE/Thrift/BE selection, or keep rejecting configurations the query protocol
cannot select uniquely.
##########
be/src/storage/index/inverted/token_filter/word_delimiter_filter.h:
##########
@@ -106,20 +120,29 @@ class WordDelimiterConcatenation {
void append(const char* text, int32_t offset, int32_t length) {
_buffer.append(text, offset, length);
+ auto source_byte_offsets = _filter.slice_source_byte_offsets(offset,
offset + length);
+ if (!_source_byte_offsets.empty() && !source_byte_offsets.empty()) {
+ _source_byte_offsets.insert(_source_byte_offsets.end(),
source_byte_offsets.begin() + 1,
Review Comment:
[P1] Preserve source gaps when catenating delimiter-separated parts
This drops the first boundary of each later slice, forcing one shared
boundary to represent both the end before a removed delimiter and the start
after it. In the valid `keyword -> word_delimiter(catenate_words=true) ->
pinyin(ignore_pinyin_offset=false)` chain, `liu-de` supplies `[0,1,2,3]` and
`[4,5,6]`, but the concatenated `liude` gets `[0,1,2,3,5,6]`; Pinyin accepts
that size-valid map and reports `de` as `[3,6)` instead of `[4,6)`.
`catenate_all` has the same problem across a removed gap. This is distinct from
the existing contiguous-subterm and direct-Pinyin-gap threads because this
concatenation itself creates a non-contiguous token. Please preserve per-rune
source intervals (or reject this ordering) and cover delimiter-separated
catenation plus reset/reuse.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -154,43 +203,71 @@ private static String resolveComponentIdentity(String
name, IndexPolicyTypeEnum
return "";
}
- // Check if it's a built-in component
- if (expectedType == IndexPolicyTypeEnum.TOKENIZER
- && IndexPolicy.BUILTIN_TOKENIZERS.contains(name)) {
- return name;
- }
-
- // For custom component, get its properties
+ // Existing named policies take precedence over built-ins for upgrade
compatibility.
try {
Env env = Env.getCurrentEnv();
- if (env == null || env.getIndexPolicyMgr() == null) {
- return name;
- }
-
- IndexPolicy policy = env.getIndexPolicyMgr().getPolicyByName(name);
- if (policy == null || policy.getType() != expectedType) {
- return name;
- }
- if (policy.isInvalid()) {
- return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ if (env != null && env.getIndexPolicyMgr() != null) {
+ IndexPolicy policy =
env.getIndexPolicyMgr().getPolicyByName(name);
+ if (policy != null && policy.getType() == expectedType) {
+ if (policy.isInvalid()) {
+ return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ }
+ Map<String, String> props = policy.getProperties();
+ if (props != null && !props.isEmpty()) {
+ TreeMap<String, String> sortedProps = new
TreeMap<>(props);
+ String type = sortedProps.get(IndexPolicy.PROP_TYPE);
+ String normalizedType =
normalizeBuiltinComponentName(type, expectedType);
+ if (normalizedType != null) {
+ if ("empty".equals(normalizedType)) {
+ return "";
+ }
+ if (sortedProps.size() == 1) {
+ return normalizedType;
+ }
+ sortedProps.put(IndexPolicy.PROP_TYPE,
normalizedType);
+ }
+ if (expectedType == IndexPolicyTypeEnum.TOKENIZER
+ &&
"ngram".equals(sortedProps.get(IndexPolicy.PROP_TYPE))) {
+ // This setting only limits policy creation; it
does not change emitted tokens.
+ sortedProps.remove(PROP_MAX_NGRAM_DIFF);
+ }
+ if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER
+ &&
"char_replace".equals(sortedProps.get(IndexPolicy.PROP_TYPE))) {
+ String replacement =
sortedProps.getOrDefault("replacement", " ");
+ String pattern = canonicalizeCharReplacePattern(
Review Comment:
[P1] Canonicalize named char filters in their tokenizer context
Passing `false` here canonicalizes a named `char_replace` only as an
isolated byte set. A named `A -> a` filter therefore makes
`{tokenizer=ik_smart}` and `{tokenizer=ik_smart,char_filter=cf}` different
identities, although BE applies the filter before the IK factory's mandatory
lowercase pass, so their terms, positions, and offsets are identical. The
outer-filter path already folds this exact IK-absorbed mapping; the earlier
named-filter thread covers only order, duplicates, and self-replacements, not
this downstream composition. Please canonicalize the completed analyzer
pipeline (or pass the resolved tokenizer context here) and cover both CREATE
and ALTER.
--
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]
