airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4002560513
##########
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:
Implemented in 6de2d54e. ICUNormalizerCharFilter now builds the
destination-byte to source-byte boundary table once from the ICU fine edits
during fill(), so correct_offset() is O(1) and still composes through nested
char filters. The ASAN PinyinFilter coverage includes 4,096 fullwidth
normalized tokens and a reset check (54 focused tests pass).
##########
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:
Implemented in 6de2d54e. Legacy and built-in IK now resolve their canonical
tokenizer base first and only then append the outer char-filter suffix. Unit
and regression coverage verify that legacy IK smart and the equivalent named
analyzer with the same outer filter are rejected as duplicates in both CREATE
and ALTER paths.
##########
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:
Implemented in 6de2d54e. Canonicalization removes A-to-a only for the
resolved default-lowercase built-in IK base. The new tests cover the duplicate
case, preserve distinction for lower_case=false, and preserve distinction for a
named analyzer whose tokenizer shadows the built-in name.
##########
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:
Implemented in 6de2d54e. The identity now canonicalizes char_replace exactly
as the BE bitset does: it sorts and deduplicates pattern bytes, removes the
replacement byte as a no-op, and omits an empty resulting filter. Unit coverage
verifies equivalent reordered/duplicate patterns produce the same identity.
--
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]
