airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4067853653
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -150,49 +205,287 @@ 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);
+ canonicalizePinyinDependencies(properties);
+ return;
+ }
+
+ if (expectedType == IndexPolicyTypeEnum.TOKEN_FILTER) {
+ if ("asciifolding".equals(type)) {
+ removeBooleanDefaults(properties, false, "preserve_original");
+ } else if ("word_delimiter".equals(type)) {
Review Comment:
Fixed in 31d36ebc317. Confirmed against the BE factories: `token_chars` and
`protected_words` go through `Settings::get_word_set` (an unordered set of
trimmed words), `tokenize_on_chars` is collected into an
`unordered_set<UChar32>`, `type_table` is folded into a per-code-point map
where the last rule wins, and `custom_token_chars` becomes an ICU `UnicodeSet`.
The identity now serializes these from their effective values: word sets are
trimmed, deduplicated and sorted; `custom_token_chars` is emitted as sorted
distinct code points; `tokenize_on_chars` entries are deduplicated and sorted;
`type_table` is reduced to one rule per code point in code-point order (rules
containing escapes keep their raw identity rather than re-implementing BE
unescaping on FE). Added identity cases for reordered/duplicated values in
`AnalyzerIdentityBuilderTest`, shared CREATE validation cases in
`InvertedIndexPropertiesTest`, and an ALTER duplicate-rejection case in
`SchemaChangeHandlerTest`.
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -3344,6 +3344,12 @@ private boolean checkDuplicateIndexes(List<Index>
indexes, IndexDefinition index
Column column = olapTable.getColumn(columnName);
if (column != null && (column.getType().isStringType() ||
column.getType().isVariantType())) {
if (index.getIndexType() == IndexType.INVERTED) {
+ if (InvertedIndexUtil.hasSameNonIkAnalyzerSelector(
Review Comment:
Fixed in 31d36ebc317. Reproduced first: with the fix reverted, `ALTER TABLE
... ADD INDEX ... PROPERTIES("analyzer"="IK")` persisted `IK` in the catalog
index (`expected: <ik> but was: <IK>`). `CreateIndexOp.validate()` built the
catalog `Index` (whose constructor copies the property map) before
`checkColumn()` normalized the definition, and `is_builtin_analyzer` on BE is
an exact string compare, so the persisted `IK` would be looked up as a custom
policy.
`CreateIndexOp.validate()` now resolves the analyzer/normalizer names right
after `indexDef.validate()`, so the duplicate comparison and the catalog
translation both see the canonical spelling.
`testAddInvertedIndexStoresCanonicalBuiltinAnalyzer` covers the persisted
property, the mixed-case ALTER duplicate against an existing index and within
one statement, and the CREATE path.
--
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]
