airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4069258598


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -150,47 +221,433 @@ 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, null);
+    }
+
+    /**
+     * {@code foldBlockedBytes} is the case-folding context of a char filter: 
null without a
+     * downstream fold, otherwise the bytes that filters between this one and 
the fold rewrite.
+     */
+    private static String resolveComponentIdentity(
+            String name, IndexPolicyTypeEnum expectedType, boolean[] 
foldBlockedBytes) {
         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, 
foldBlockedBytes);
+                            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",

Review Comment:
   Fixed in 7345ff82676. Confirmed on BE: `pinyin_filter.cpp` reads 
`keepNoneChineseInJoinedFullPinyin` only to append to `full_pinyin_buffer`, and 
that buffer is emitted only behind `keepJoinedFullPinyin`. Reproduced first: a 
type-only Pinyin token filter and the same policy with 
`keep_none_chinese_in_joined_full_pinyin=true` had different identities 
(`{keep_none_chinese_in_joined_full_pinyin=true, type=pinyin}` vs `pinyin`).
   
   The identity now drops the setting whenever the effective 
`keep_joined_full_pinyin` is false, with one deliberate exception: the pinyin 
*tokenizer* (`pinyin_tokenizer.cpp` `parseBuff`) also consults the flag to 
decide whether an untokenized ASCII buffer is emitted when 
`none_chinese_pinyin_tokenize=false` and `keep_first_letter`, 
`keep_separate_first_letter` and `keep_full_pinyin` are all off, so that corner 
keeps the setting. `testPinyinNoneChineseInJoinedFullPinyinFollowsJoinedGate` 
covers both component forms plus the tokenizer corner; CREATE and ALTER 
duplicate rejection are in 
`testCreateTableRejectsPinyinSettingsBehindDisabledGates` and 
`testAddInvertedIndexRejectsPinyinSettingsBehindDisabledGates`.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -226,26 +681,321 @@ 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) {
+        ArrayDeque<String> identities = new ArrayDeque<>();
+        walkCharFilters(filterList, lowercaseDownstream, identities);
+        return String.join(",", identities);
+    }
+
+    /**
+     * Resolve the chain from its last filter to its first, collecting 
identities, and return the
+     * case-folding context that a filter placed in front of the chain would 
run in.
+     */
+    private static boolean[] walkCharFilters(
+            String filterList, boolean lowercaseDownstream, Deque<String> 
identities) {
+        boolean[] foldBlockedBytes = lowercaseDownstream ? new boolean[256] : 
null;
         if (Strings.isNullOrEmpty(filterList)) {
-            return "";
+            return foldBlockedBytes;
         }
 
-        StringBuilder sb = new StringBuilder();
         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, 
foldBlockedBytes);
+            if (Strings.isNullOrEmpty(filter)) {
+                continue;
             }
+            identities.addFirst(filter);
+            foldBlockedBytes = foldBlockedBytesBefore(filterName, 
foldBlockedBytes);
+        }
+        return foldBlockedBytes;
+    }
 
-            if (IndexPolicy.BUILTIN_CHAR_FILTERS.contains(filter)) {
-                sb.append(filter);
-            } else {
-                sb.append(resolveComponentIdentity(filter, 
IndexPolicyTypeEnum.CHAR_FILTER));
+    /**
+     * Context for the filter that runs before this one: a case fold starts a 
fresh context, a
+     * char_replace filter adds the bytes it rewrites, and any other filter 
ends the context.
+     */
+    private static boolean[] foldBlockedBytesBefore(String filterName, 
boolean[] foldBlockedBytes) {
+        if (isCaseFoldingCharFilter(filterName)) {
+            return new boolean[256];
+        }
+        if (foldBlockedBytes == null) {
+            return null;
+        }
+        boolean[] sourceBytes = charReplaceSourceBytes(filterName);
+        if (sourceBytes == null) {
+            return null;
+        }
+        for (int i = 0; i < foldBlockedBytes.length; ++i) {
+            foldBlockedBytes[i] |= sourceBytes[i];
+        }
+        return foldBlockedBytes;
+    }
+
+    /** Bytes a named char_replace filter rewrites, or null for any other 
filter. */
+    private static boolean[] charReplaceSourceBytes(String filterName) {
+        IndexPolicy policy = findPolicy(filterName, 
IndexPolicyTypeEnum.CHAR_FILTER);
+        if (policy == null || policy.isInvalid() || policy.getProperties() == 
null) {
+            return null;
+        }
+        Map<String, String> properties = policy.getProperties();
+        String type = normalizeBuiltinComponentName(
+                properties.get(IndexPolicy.PROP_TYPE), 
IndexPolicyTypeEnum.CHAR_FILTER);
+        String pattern = properties.get("pattern");
+        if (!"char_replace".equals(type) || pattern == null) {
+            return null;
+        }
+        boolean[] sourceBytes = new boolean[256];
+        for (int i = 0; i < pattern.length(); ++i) {
+            char patternByte = pattern.charAt(i);
+            if (patternByte < sourceBytes.length) {
+                sourceBytes[patternByte] = true;
             }
         }
-        return sb.toString();
+        return sourceBytes;
+    }
+
+    /** The named policy when one exists with the expected type, or null. */
+    private static IndexPolicy findPolicy(String name, IndexPolicyTypeEnum 
expectedType) {
+        if (Strings.isNullOrEmpty(name)) {
+            return null;
+        }
+        try {
+            Env env = Env.getCurrentEnv();
+            if (env != null && env.getIndexPolicyMgr() != null) {
+                IndexPolicy policy = 
env.getIndexPolicyMgr().getPolicyByName(name);
+                if (policy != null && policy.getType() == expectedType) {
+                    return policy;
+                }
+            }
+        } catch (RuntimeException e) {
+            // Treat lookup failures as an unknown policy.
+        }
+        return null;
+    }
+
+    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);

Review Comment:
   Fixed in 7345ff82676. Confirmed on BE: `ICUNormalizerCharFilterFactory` 
parses `unicode_set_filter` and uses the base normalizer when the parsed set is 
empty, so `[]` is the unfiltered `nfkc_cf`. Reproduced first: 
`lower_a,fold_empty` kept `{pattern=A, ...}` and the outer `A -> a` suffix 
while `fold_empty` did not.
   
   `isCaseFoldingCharFilter` now goes through 
`isCaseFoldingIcuNormalizer`/`isUnfilteredIcuNormalizer`, which treat a missing 
filter, an empty string, and any pattern that parses to an empty `UnicodeSet` 
as unfiltered (an unparsable pattern is not a fold). The same helper feeds the 
token-filter fold scan from the next threads, and 
`canonicalizeIcuNormalizerDefaults` now also drops an explicit empty-string 
filter so it matches an absent one. `testEmptyUnicodeSetIcuNormalizerFoldsCase` 
covers `[]` with `[b]` and an unparsable `[b` as negatives; CREATE is in 
`testCreateTableRejectsEmptyUnicodeSetFoldAliases` and ALTER in 
`testAddInvertedIndexRejectsFoldAliasesThroughEmptySetNormalizerAndTransparentFilters`.



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