github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4064074510


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -150,47 +204,207 @@ 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;
-        }
-
-        // 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;
+            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);
+            return;
+        }
+
+        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");
+            } else if ("icu_normalizer".equals(type)) {
+                canonicalizeIcuNormalizerDefaults(properties, false);
             }
+            return;
+        }
 
-            IndexPolicy policy = env.getIndexPolicyMgr().getPolicyByName(name);
-            if (policy == null || policy.getType() != expectedType) {
-                return name;
+        if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER) {
+            if ("icu_normalizer".equals(type)) {
+                canonicalizeIcuNormalizerDefaults(properties, true);
             }
-            if (policy.isInvalid()) {
-                return "invalid-policy:" + policy.getId() + ":" + 
policy.getName();
+            return;
+        }
+
+        if (expectedType != IndexPolicyTypeEnum.TOKENIZER) {
+            return;
+        }
+        switch (type) {
+            case "ngram":
+                removeIntegerDefault(properties, "min_gram", 1);
+                removeIntegerDefault(properties, "max_gram", 2);
+                break;
+            case "edge_ngram":
+                removeIntegerDefault(properties, "min_gram", 1);
+                removeIntegerDefault(properties, "max_gram", 2);
+                break;
+            case "standard":
+            case "char_group":
+                removeIntegerDefault(properties, "max_token_length", 255);
+                break;
+            case "keyword":
+                removeIntegerDefault(properties, "buffer_size", 256);
+                break;
+            case "basic":
+                removeStringDefault(properties, "extra_chars", "");

Review Comment:
   [P1] Canonicalize Basic's effective extra-character set
   
   This keeps a nonempty `extra_chars` value verbatim, although BE's 
`BasicTokenizer::initialize()` reduces it to a byte membership set. 
Consequently policies with `extra_chars=+-` and `extra_chars=-+` produce 
identical terms, positions, and offsets but different analyzer identities, so 
indexes using differently named analyzers can bypass the new semantic-duplicate 
check. Please sort/deduplicate this property and cover reordered/duplicated 
values in identity plus CREATE/ALTER tests.



-- 
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]

Reply via email to