airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4068729104
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -226,26 +671,115 @@ private static String resolveTokenFilterIdentity(String
filterList) {
* IMPORTANT: Order is preserved because filter order is semantically
significant.
*/
private static String resolveCharFilterIdentity(String filterList) {
+ return resolveCharFilterIdentity(filterList, false);
+ }
+
+ private static String resolveCharFilterIdentity(String filterList, boolean
lowercaseDownstream) {
if (Strings.isNullOrEmpty(filterList)) {
return "";
}
- StringBuilder sb = new StringBuilder();
+ ArrayDeque<String> identities = new ArrayDeque<>();
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 (int i = filters.length - 1; i >= 0; --i) {
+ String filterName = filters[i].trim();
+ String filter = resolveComponentIdentity(
+ filterName, IndexPolicyTypeEnum.CHAR_FILTER,
lowercaseDownstream);
+ if (Strings.isNullOrEmpty(filter)) {
+ continue;
}
+ identities.addFirst(filter);
+ lowercaseDownstream = isCaseFoldingCharFilter(filterName);
+ }
+ return String.join(",", identities);
+ }
- if (IndexPolicy.BUILTIN_CHAR_FILTERS.contains(filter)) {
- sb.append(filter);
- } else {
- sb.append(resolveComponentIdentity(filter,
IndexPolicyTypeEnum.CHAR_FILTER));
+ private static boolean isCaseFoldingCharFilter(String name) {
+ if (Strings.isNullOrEmpty(name)) {
+ return false;
+ }
+
+ try {
+ Env env = Env.getCurrentEnv();
+ if (env != null && env.getIndexPolicyMgr() != null) {
+ IndexPolicy policy =
env.getIndexPolicyMgr().getPolicyByName(name);
+ if (policy != null && policy.getType() ==
IndexPolicyTypeEnum.CHAR_FILTER) {
+ if (policy.isInvalid()) {
+ return false;
+ }
+ Map<String, String> properties = policy.getProperties();
+ if (properties != null && !properties.isEmpty()) {
+ String type = normalizeBuiltinComponentName(
+ properties.get(IndexPolicy.PROP_TYPE),
IndexPolicyTypeEnum.CHAR_FILTER);
+ String normalizer = properties.getOrDefault("name",
"nfkc_cf").trim();
+ String unicodeSet =
properties.getOrDefault("unicode_set_filter", "").trim();
+ return "icu_normalizer".equals(type)
+ && "nfkc_cf".equalsIgnoreCase(normalizer)
+ && unicodeSet.isEmpty();
+ }
+ }
+ }
+ } catch (RuntimeException e) {
+ // Fall through to built-in resolution.
+ }
+
+ return "icu_normalizer".equals(
+ normalizeBuiltinComponentName(name,
IndexPolicyTypeEnum.CHAR_FILTER));
+ }
+
+ 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, " ");
+ String canonicalPattern = canonicalizeCharReplacePattern(
+ pattern, replacement,
isDefaultLowercaseBuiltinIkIdentity(analyzerIdentity));
Review Comment:
Fixed in f5d7670ec2f. Reproduced first: two aliases of `tokenizer=keyword,
token_filter=lowercase`, one with outer `char_replace` `A -> a`, got different
identities (`|outer_char_filter=char_replace:1:A:1:a;` only on one) and were
admitted by CREATE and ALTER.
`appendOuterCharFilterIdentity` now receives the fold context of the
analyzer the outer filter runs in front of (BE applies it before the analyzer's
own char filters). For a custom analyzer that context is derived from its
pipeline: the mapping is absorbed only when the tokenizer is case transparent
(standard, keyword, icu, basic, ngram/edge_ngram without `custom_token_chars`,
char_group whose `tokenize_on_chars` names no ASCII letter) and the first
effective token filter is `lowercase`, or when the tokenizer is IK, and the
analyzer's own char filter chain is walked with the same blocked-bytes rule as
the previous thread. Anything else keeps the suffix.
`testOuterCharFilterAbsorbedByCustomCaseFoldingPipeline` covers the equivalent
aliases with and without `A -> a`, plus the no-lowercase, `char_group [A]` and
`ngram custom_token_chars` negatives; CREATE and ALTER duplicate rejection are
covered in `InvertedIndexPropertiesTest` and `SchemaChangeHandlerTest`.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -150,49 +216,430 @@ private static String
buildIdentityFromPolicyProperties(IndexPolicyTypeEnum type
* Resolve a component (tokenizer) to its identity.
*/
private static String resolveComponentIdentity(String name,
IndexPolicyTypeEnum expectedType) {
+ return resolveComponentIdentity(name, expectedType, false);
+ }
+
+ private static String resolveComponentIdentity(
+ String name, IndexPolicyTypeEnum expectedType, boolean
lowercaseDownstream) {
if (Strings.isNullOrEmpty(name)) {
return "";
}
- // Check if it's a built-in component
- if (expectedType == IndexPolicyTypeEnum.TOKENIZER
- && IndexPolicy.BUILTIN_TOKENIZERS.contains(name)) {
- return name;
+ // Existing named policies take precedence over built-ins for upgrade
compatibility.
+ try {
+ Env env = Env.getCurrentEnv();
+ 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 "";
+ }
+ sortedProps.put(IndexPolicy.PROP_TYPE,
normalizedType);
+ canonicalizeEffectiveComponentProperties(
+ sortedProps, normalizedType, expectedType);
+ if (sortedProps.size() == 1) {
+ return 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(
+ sortedProps.get("pattern"), replacement,
lowercaseDownstream);
+ if (pattern.isEmpty()) {
+ return "";
+ }
+ sortedProps.put("pattern", pattern);
+ sortedProps.put("replacement", replacement);
+ }
+ if (normalizedType != null && sortedProps.size() == 1)
{
+ return normalizedType;
+ }
+ return sortedProps.toString();
+ }
+ }
+ }
+ } catch (RuntimeException e) {
+ // Fall through to built-in resolution or the original name.
+ }
+
+ String normalizedName = normalizeBuiltinComponentName(name,
expectedType);
+ return "empty".equals(normalizedName) ? "" : normalizedName == null ?
name : normalizedName;
+ }
+
+ private static void canonicalizeEffectiveComponentProperties(
+ TreeMap<String, String> properties, String type,
IndexPolicyTypeEnum expectedType) {
+ if ("pinyin".equals(type)) {
+ removeBooleanDefaults(properties, true,
+ "keep_first_letter", "keep_full_pinyin",
"keep_none_chinese",
+ "keep_none_chinese_together",
"keep_none_chinese_in_first_letter",
+ "lowercase", "trim_whitespace", "ignore_pinyin_offset",
+ "none_chinese_pinyin_tokenize");
+ removeBooleanDefaults(properties, false,
+ "keep_separate_first_letter", "keep_joined_full_pinyin",
"keep_original",
+ "keep_none_chinese_in_joined_full_pinyin",
"remove_duplicated_term",
+ "fixed_pinyin_offset", "keep_separate_chinese");
+ removeIntegerDefault(properties, "limit_first_letter_length", 16);
+ canonicalizePinyinDependencies(properties);
+ return;
}
- // For custom component, get its properties
+ if (expectedType == IndexPolicyTypeEnum.TOKEN_FILTER) {
+ if ("asciifolding".equals(type)) {
+ removeBooleanDefaults(properties, false, "preserve_original");
+ } else if ("word_delimiter".equals(type)) {
+ removeBooleanDefaults(properties, true, "generate_word_parts",
"generate_number_parts",
+ "split_on_case_change", "split_on_numerics",
"stem_english_possessive");
+ removeBooleanDefaults(properties, false, "catenate_words",
"catenate_numbers",
+ "catenate_all", "preserve_original");
+ canonicalizeWordSet(properties, "protected_words");
+ canonicalizeTypeTable(properties);
+ } else if ("icu_normalizer".equals(type)) {
+ canonicalizeIcuNormalizerDefaults(properties, false);
+ }
+ return;
+ }
+
+ if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER) {
+ if ("icu_normalizer".equals(type)) {
+ canonicalizeIcuNormalizerDefaults(properties, true);
+ }
+ return;
+ }
+
+ if (expectedType != IndexPolicyTypeEnum.TOKENIZER) {
+ return;
+ }
+ switch (type) {
+ case "ngram":
+ case "edge_ngram":
+ removeIntegerDefault(properties, "min_gram", 1);
+ removeIntegerDefault(properties, "max_gram", 2);
+ canonicalizeWordSet(properties, "token_chars");
+ canonicalizeCustomTokenChars(properties);
+ break;
+ case "standard":
+ removeIntegerDefault(properties, "max_token_length", 255);
+ break;
+ case "char_group":
+ removeIntegerDefault(properties, "max_token_length", 255);
+ canonicalizeTokenizeOnChars(properties);
+ break;
+ case "keyword":
+ removeIntegerDefault(properties, "buffer_size", 256);
Review Comment:
Fixed in f5d7670ec2f. Confirmed on BE: `KeywordTokenizer::initialize()` only
range-checks `buffer_size`, and `next()` always clips with the constant
`MAX_TOKEN_LENGTH_LIMIT`, so every accepted value emits the same term, offsets
and provenance. Reproduced first: a replayed keyword policy with
`buffer_size=512` had the identity `ANALYZER:tokenizer={buffer_size=512,
type=keyword};` and both CREATE and ALTER admitted it next to a 256 alias.
The keyword branch now removes `buffer_size` from the identity regardless of
its value. `testKeywordBufferSizeDoesNotChangeIdentity`,
`testCreateTableRejectsIneffectiveKeywordBufferSizeAliases` and the ALTER case
in `SchemaChangeHandlerTest` cover 256 versus 512 (the policies are replayed,
since the FE validator only accepts `type` for keyword tokenizers).
--
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]
