github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4001459129
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -152,35 +182,49 @@ private static String resolveComponentIdentity(String
name, IndexPolicyTypeEnum
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;
- }
-
- IndexPolicy policy = env.getIndexPolicyMgr().getPolicyByName(name);
- if (policy == null || policy.getType() != expectedType) {
- return name;
+ if (env != null && env.getIndexPolicyMgr() != null) {
+ IndexPolicy policy =
env.getIndexPolicyMgr().getPolicyByName(name);
+ if (policy != null && policy.getType() == expectedType) {
+ 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 (sortedProps.size() == 1) {
+ return normalizedType;
+ }
+ sortedProps.put(IndexPolicy.PROP_TYPE,
normalizedType);
+ }
+ return sortedProps.toString();
+ }
+ }
}
+ } catch (RuntimeException e) {
+ // Fall through to built-in resolution or the original name.
+ }
- Map<String, String> props = policy.getProperties();
- if (props == null || props.isEmpty()) {
- return name;
- }
+ String normalizedName = normalizeBuiltinComponentName(name,
expectedType);
+ return normalizedName == null ? name : normalizedName;
+ }
- // Build identity from sorted properties
- TreeMap<String, String> sortedProps = new TreeMap<>(props);
- return sortedProps.toString();
- } catch (RuntimeException e) {
- return name;
+ private static String normalizeBuiltinComponentName(String name,
IndexPolicyTypeEnum expectedType) {
+ if (Strings.isNullOrEmpty(name)) {
+ return null;
+ }
+ String normalizedName = name.toLowerCase(Locale.ROOT);
Review Comment:
[P1] Trim component names when building analyzer identity
FE validation already trims component references, and BE trims again before
factory lookup, so an analyzer policy with `tokenizer = " IK_SMART "` is
accepted and executes exactly like `ik_smart`. This helper only lowercases,
however, so the padded spelling falls through to its raw identity. CREATE and
ALTER can then accept two equivalent inverted indexes on the same column.
Please canonicalize with the same trim-plus-`Locale.ROOT` rule used by
validation/runtime resolution and cover padded IK references in both
duplicate-check paths.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -50,9 +52,37 @@ public static String buildAnalyzerIdentity(
if (Strings.isNullOrEmpty(parser) ||
parserNone.equalsIgnoreCase(parser)) {
return defaultAnalyzerKey;
}
+ String legacyIkIdentity = resolveLegacyIkIdentity(properties, parser);
Review Comment:
[P1] Canonicalize the default built-in IK analyzer with max-word
The new equivalence handles `parser=ik, parser_mode=ik_max_word`, but the
other built-in spelling `analyzer=ik` still returns the literal identity `ik`.
On BE the default, no-override form parses as `PARSER_IK`; its mode is
`coarse_grained`, so `create_builtin_analyzer` takes the non-smart branch and
produces the same stream as a custom `ik_max_word` tokenizer. The two
runtime-equivalent indexes therefore evade the duplicate fence. Please map the
behavior-equivalent default `analyzer=ik` form to the same synthetic max-word
identity (retaining guards for `lower_case`/char-filter overrides), and add
CREATE/ALTER coverage for this pair.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java:
##########
@@ -61,7 +62,7 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {
* Policy names are case-insensitive in Doris.
*/
private static String normalizeKey(String name) {
- return name == null ? null : name.trim().toLowerCase();
+ return name == null ? null : name.trim().toLowerCase(Locale.ROOT);
Review Comment:
[P1] Preserve surviving policies across locale-key migration
Before this change, a Turkish-default FE treats `IK_SMART` and `ik_smart` as
distinct keys (dotless versus dotted i), so both names can be legally
persisted. Consider the old journal history create A, create B, then drop A:
old replay leaves B reachable. With `Locale.ROOT`, replay first maps A and B to
the same `ik_smart` key, then `replayDropIndexPolicy(A)` removes that shared
key unconditionally, leaving B in `idToIndexPolicy` but unreachable by name
(and image reconstruction has the same collision class). Please migrate/detect
old normalization-key collisions deterministically and make replayed removal
conditional on the mapped policy ID, with image and journal coverage for this
history.
--
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]
