github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4066332800
##########
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:
[P1] Canonicalize collection-valued component settings before building this
identity. For example, FE accepts NGram/EdgeNGram `token_chars="letter,digit"`
and `token_chars="digit,letter,letter"`, but BE parses both into the same
unordered matcher set; CharGroup `tokenize_on_chars` and WordDelimiter
`protected_words` are likewise reduced to sets, and `type_table` becomes an
effective per-code-point map. Because this switch only normalizes scalar
defaults, aliases with reordered/duplicated collection values get different
identities even though they emit the same terms, positions, and offsets, so
both CREATE and ALTER can admit the duplicate index. Please serialize these
fields from their effective parsed sets/maps (including `custom_token_chars`)
and cover reordered/duplicated values in the identity and both DDL paths.
##########
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)) {
+ 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;
+ }
+
+ if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER) {
+ if ("icu_normalizer".equals(type)) {
+ canonicalizeIcuNormalizerDefaults(properties, true);
}
+ 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":
+ canonicalizeBasicExtraChars(properties);
+ break;
+ default:
+ break;
+ }
+ }
- IndexPolicy policy = env.getIndexPolicyMgr().getPolicyByName(name);
- if (policy == null || policy.getType() != expectedType) {
- return name;
+ private static void removeBooleanDefaults(
+ TreeMap<String, String> properties, boolean defaultValue,
String... keys) {
+ for (String key : keys) {
+ String value = properties.get(key);
+ if (value == null || !("true".equalsIgnoreCase(value) ||
"false".equalsIgnoreCase(value))) {
+ continue;
}
- if (policy.isInvalid()) {
- return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ boolean parsed = Boolean.parseBoolean(value);
+ if (parsed == defaultValue) {
+ properties.remove(key);
+ } else {
+ properties.put(key, Boolean.toString(parsed));
}
+ }
+ }
- Map<String, String> props = policy.getProperties();
- if (props == null || props.isEmpty()) {
- return name;
+ private static void removeIntegerDefault(
+ TreeMap<String, String> properties, String key, int defaultValue) {
+ String value = properties.get(key);
+ if (value == null) {
+ return;
+ }
+ try {
+ int parsed = Integer.parseInt(value);
+ if (parsed == defaultValue) {
+ properties.remove(key);
+ } else {
+ properties.put(key, Integer.toString(parsed));
}
+ } catch (NumberFormatException e) {
+ // Invalid policies keep their original identity.
+ }
+ }
- // Build identity from sorted properties
- TreeMap<String, String> sortedProps = new TreeMap<>(props);
- 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);
+ private static void canonicalizeIcuNormalizerDefaults(
+ TreeMap<String, String> properties, boolean hasMode) {
+ String name = properties.get("name");
+ if (name != null) {
+ String normalizedName = name.trim().toLowerCase(Locale.ROOT);
+ if ("nfkc_cf".equals(normalizedName)) {
+ properties.remove("name");
+ } else {
+ properties.put("name", normalizedName);
}
- return sortedProps.toString();
- } catch (RuntimeException e) {
- return name;
+ }
+ String filter = properties.get("unicode_set_filter");
+ if (filter != null) {
+ try {
+ UnicodeSet unicodeSet = new UnicodeSet(filter);
+ if (unicodeSet.isEmpty()) {
+ properties.remove("unicode_set_filter");
+ } else {
+ properties.put("unicode_set_filter",
unicodeSet.toPattern(false));
+ }
+ } catch (IllegalArgumentException e) {
+ // Invalid policies keep their original identity.
+ }
+ }
+ if (hasMode) {
+ removeStringDefault(properties, "mode", "compose");
Review Comment:
[P1] Canonicalize `mode` according to the selected ICU normalizer. FE
accepts both values for every name, but BE ignores `mode` for `nfd` and `nfkd`
and always returns the corresponding decomposition instance. Consequently
`{name=nfd}` (or explicit `mode=compose`, which this method removes) and
`{name=nfd,mode=decompose}` emit identical text and offsets but retain
different identities, so aliases can bypass both CREATE and ALTER duplicate
rejection. Drop the irrelevant mode for NFD/NFKD (or reject the ineffective
combinations) and add identity plus both DDL-path cases.
##########
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:
[P1] Run analyzer canonicalization before this ALTER duplicate check and
before the catalog `Index` is snapshotted. `CreateIndexOp.validate()` has
already copied the raw property map into `alterIndex`, while
`indexDef.checkColumn()` runs only after these checks and never copies its
normalized properties back. Thus, with an existing `analyzer=ik` index, `ALTER
... analyzer=IK` gets a distinct raw identity (the IK selector exemption also
skips the selector collision), and the persisted `Index` still says `IK`. The
new BE built-in dispatch is case-sensitive, so it treats that as a custom
policy and fails writer setup with `Policy not found with name: IK`. Please
validate/canonicalize before duplicate comparison and catalog translation (or
resynchronize `alterIndex` afterward), with an ALTER mixed-case duplicate and
serialized-property test.
##########
be/src/storage/index/inverted/tokenizer/tokenizer.h:
##########
@@ -39,12 +45,131 @@ class DorisTokenizer : public Tokenizer, public
DorisTokenStream {
using Tokenizer::reset;
// Only use the parameterless reset method
- void reset() override { _in = _in_pending; };
+ void reset() override {
+ _in = _in_pending;
+ _source_byte_offsets.clear();
Review Comment:
[P1] Apply the provenance high-water cleanup to the reusable tokenizer too.
An offset-aware `empty -> pinyin(ignore_pinyin_offset=false)` chain allocates
one `int32_t` boundary per input rune here, so a 100 MiB ASCII value leaves
about 400 MiB in `_source_byte_offsets`; `clear()` keeps that capacity for the
cached stream's remaining writer lifetime. The Pinyin reset fix releases only
its copied snapshots, not this upstream allocation, and later skipped/empty
values need not replace it. Release oversized
`_source_byte_offsets`/`_source_byte_end_offsets` on reset while retaining
ordinary capacities, and cover a large-token reset followed by no-token and
small-token reuse.
##########
be/src/storage/index/inverted/token_filter/word_delimiter_filter.cpp:
##########
@@ -175,6 +195,11 @@ void WordDelimiterFilter::reset() {
_buffered_pos = 0;
_buffered_len = 0;
_first = true;
+ _saved_source_byte_offsets.clear();
Review Comment:
[P1] Apply the provenance high-water cleanup to WordDelimiter-owned state
too. In an FE-valid `empty -> word_delimiter ->
pinyin(ignore_pinyin_offset=false)` chain, one all-letter token makes
`save_source_state()` retain three dense vectors (saved starts, saved ends, and
token boundaries), then the unchanged-token path copies starts and ends into
the two current vectors. A 100 MiB ASCII value therefore leaves about 2 GiB of
capacity here after these `clear()` calls, and the cached reusable analyzer can
keep it for the writer lifetime; Pinyin's 64 KiB cleanup only releases its own
snapshots. Release oversized saved/current, buffered-attribute, state, and
concatenation provenance capacity while retaining ordinary reuse, and cover a
large whole token followed by empty and small resets.
--
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]
