github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4002360515
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -43,14 +45,63 @@ 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);
+ return appendOuterCharFilterIdentity(
+ resolveAnalyzerIdentity(preferredAnalyzer,
defaultAnalyzerKey, log), properties);
}
if (Strings.isNullOrEmpty(parser) ||
parserNone.equalsIgnoreCase(parser)) {
return defaultAnalyzerKey;
}
- return parser;
+ String legacyIkIdentity = resolveLegacyIkIdentity(properties, parser);
+ if (legacyIkIdentity != null) {
+ return legacyIkIdentity;
+ }
+ return appendOuterCharFilterIdentity(parser, properties);
+ }
+
+ private static String resolveBuiltinIkAnalyzerIdentity(
+ Map<String, String> properties, String analyzer) {
+ // BE defaults analyzer=ik to max-word mode. It is equivalent to the
built-in
+ // ik_max_word tokenizer only when no index-level option changes its
behavior.
+ if
(!InvertedIndexProperties.INVERTED_INDEX_PARSER_IK.equalsIgnoreCase(analyzer.trim())
+ || !Strings.isNullOrEmpty(properties.get(
Review Comment:
[P1] Canonicalize the IK base before appending the outer-filter suffix. This
guard prevents the intended collapse even when both analyzers have the same
filter: for a named `smart={tokenizer=ik_smart}`, legacy
`{parser=ik,parser_mode=ik_smart,char_filter_type=char_replace,char_filter_pattern=-}`
becomes `ik|outer...`, while `{analyzer=smart}` with the identical
`char_replace` becomes `ANALYZER:tokenizer=ik_smart;|outer...`. BE applies the
same reader filter before the same smart/lowercase IK tokenizer, so CREATE and
ALTER can admit duplicate indexes. Keep filtered versus unfiltered identities
distinct by appending the suffix after canonicalizing the IK base; apply the
same rule to built-in `analyzer=ik` versus named `ik_max_word`, with both
duplicate-path tests.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -224,18 +293,30 @@ private static String resolveCharFilterIdentity(String
filterList) {
String[] filters = filterList.split(",\\s*");
// DO NOT sort - filter order is semantically significant
- for (int i = 0; i < filters.length; i++) {
- String filter = filters[i].trim();
- if (i > 0) {
- sb.append(",");
+ for (String filterName : filters) {
+ String filter = resolveComponentIdentity(filterName.trim(),
IndexPolicyTypeEnum.CHAR_FILTER);
+ if (Strings.isNullOrEmpty(filter)) {
+ continue;
}
-
- if (IndexPolicy.BUILTIN_CHAR_FILTERS.contains(filter)) {
- sb.append(filter);
- } else {
- sb.append(resolveComponentIdentity(filter,
IndexPolicyTypeEnum.CHAR_FILTER));
+ if (sb.length() > 0) {
+ sb.append(",");
}
+ sb.append(filter);
}
return sb.toString();
}
+
+ private static String appendOuterCharFilterIdentity(
+ String analyzerIdentity, Map<String, String> properties) {
+ String type =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_TYPE);
+ String pattern =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_PATTERN);
+ if (!"char_replace".equals(type) || Strings.isNullOrEmpty(pattern)) {
+ return analyzerIdentity;
+ }
+ String replacement = properties.getOrDefault(
+
InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_REPLACEMENT, " ");
+ return analyzerIdentity + "|outer_char_filter=char_replace:"
+ + pattern.length() + ":" + pattern + ":"
Review Comment:
[P1] Canonicalize the char-replacement byte set in the identity. The BE
filter stores `pattern` in a `bitset<256>`, so order and duplicates do not
affect analysis: `pattern=ab,replacement=x` and `pattern=ba,replacement=x`
produce the same stream, but these raw strings yield different identities and
allow two semantically duplicate indexes. Bytes equal to the replacement are
no-ops as well (for example `pattern=a,replacement=a` is equivalent to no outer
filter). Please serialize the effective sorted/deduplicated set (excluding
no-op bytes, and omit the filter if it becomes empty) and add equality tests
for those cases.
##########
be/src/storage/index/inverted/char_filter/icu_normalizer_char_filter.cpp:
##########
@@ -64,28 +65,31 @@ void ICUNormalizerCharFilter::fill() {
void ICUNormalizerCharFilter::normalize_text(const std::string& input,
std::string& output) {
output.clear();
+ _edits.reset();
if (input.empty()) {
return;
}
UErrorCode status = U_ZERO_ERROR;
- icu::UnicodeString src16 = icu::UnicodeString::fromUTF8(input);
- UNormalizationCheckResult quick_result = _normalizer->quickCheck(src16,
status);
- if (U_SUCCESS(status) && quick_result == UNORM_YES) {
- output = input;
- return;
- }
-
- icu::UnicodeString result16;
- status = U_ZERO_ERROR;
- _normalizer->normalize(src16, result16, status);
+ icu::StringByteSink<std::string> sink(&output);
+ _normalizer->normalizeUTF8(0, icu::StringPiece(input), sink, &_edits,
status);
if (U_FAILURE(status)) {
LOG(WARNING) << "ICU normalize failed: " << u_errorName(status) << ",
using original text";
output = input;
+ _edits.reset();
+ _edits.addUnchanged(static_cast<int32_t>(input.size()));
return;
}
+}
- result16.toUTF8String(output);
+int32_t ICUNormalizerCharFilter::correct_offset(int32_t current_offset) const {
+ UErrorCode status = U_ZERO_ERROR;
+ auto iterator = _edits.getFineIterator();
Review Comment:
[P1] Avoid restarting the ICU edit scan for every token boundary.
`getFineIterator()` starts at edit zero, and ICU 69.1's
`sourceIndexFromDestinationIndex()` advances span by span to the requested
offset. IK calls this method for each token start/end and, for offset-aware
Pinyin, each rune boundary, so an input like repeated full-width `Ａ` separated
by unchanged spaces creates alternating edit spans and O(N^2) traversal. Please
map monotonically increasing boundaries in one pass or precompute/cache
destination-to-source boundaries during normalization, and add a many-token ICU
-> IK -> Pinyin scaling/reset test.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -224,18 +293,30 @@ private static String resolveCharFilterIdentity(String
filterList) {
String[] filters = filterList.split(",\\s*");
// DO NOT sort - filter order is semantically significant
- for (int i = 0; i < filters.length; i++) {
- String filter = filters[i].trim();
- if (i > 0) {
- sb.append(",");
+ for (String filterName : filters) {
+ String filter = resolveComponentIdentity(filterName.trim(),
IndexPolicyTypeEnum.CHAR_FILTER);
+ if (Strings.isNullOrEmpty(filter)) {
+ continue;
}
-
- if (IndexPolicy.BUILTIN_CHAR_FILTERS.contains(filter)) {
- sb.append(filter);
- } else {
- sb.append(resolveComponentIdentity(filter,
IndexPolicyTypeEnum.CHAR_FILTER));
+ if (sb.length() > 0) {
+ sb.append(",");
}
+ sb.append(filter);
}
return sb.toString();
}
+
+ private static String appendOuterCharFilterIdentity(
+ String analyzerIdentity, Map<String, String> properties) {
+ String type =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_TYPE);
+ String pattern =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_PATTERN);
+ if (!"char_replace".equals(type) || Strings.isNullOrEmpty(pattern)) {
+ return analyzerIdentity;
+ }
+ String replacement = properties.getOrDefault(
+
InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_REPLACEMENT, " ");
+ return analyzerIdentity + "|outer_char_filter=char_replace:"
Review Comment:
[P1] Fold outer replacements that IK already absorbs. With default/true
lowercase IK, an outer `A -> a` mapping is behavior-neutral:
`AnalyzeContext::fillBuffer()` lowercases ASCII before classification and
segmentation, `IKTokenizer` regularizes published terms, and this one-byte
replacement preserves offsets. This suffix nevertheless makes filtered and
unfiltered smart/max IK analyzers compare unequal, so CREATE and ALTER can
admit duplicate indexes. Canonicalize this mapping against the resolved IK
base, while retaining it for `lower_case=false` or a shadowing non-IK policy,
and cover both modes and duplicate paths.
--
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]
