airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4047278959
##########
be/src/storage/index/inverted/token_filter/word_delimiter_filter.h:
##########
@@ -106,20 +120,29 @@ class WordDelimiterConcatenation {
void append(const char* text, int32_t offset, int32_t length) {
_buffer.append(text, offset, length);
+ auto source_byte_offsets = _filter.slice_source_byte_offsets(offset,
offset + length);
+ if (!_source_byte_offsets.empty() && !source_byte_offsets.empty()) {
+ _source_byte_offsets.insert(_source_byte_offsets.end(),
source_byte_offsets.begin() + 1,
Review Comment:
Fixed in `2558085d88f6e815ff6775ce0c341a83fa1baef2`.
I reproduced this with the native ASAN BE runner: the new
`PinyinFilterTest.TestWordDelimiterConcatenationPreservesSourceGapsAndReset`
failed on the previous implementation because `de` started at byte 3 instead of
4. It covers both `catenate_words` and `catenate_all`, then resets the pipeline
with another delimiter-separated input.
Concatenation now preserves independent per-rune source end offsets
alongside source starts, and Pinyin consumes those intervals. Removed
delimiters therefore no longer force the preceding end and following start to
share a boundary. The mapping remains opt-in, and reset clears both arrays. The
test now returns `liu[0,3)` and `de[4,6)` and passes on reset/reuse.
The official runner passed 178 tests across the related suites; after the
final clang-tidy corrections, all 120 tests in the three affected suites passed
again. The two custom-analyzer SQL suites and the existing IK SQL suite also
passed on the task-owned native ASAN cluster.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/InvertedIndexUtil.java:
##########
@@ -423,17 +445,18 @@ public static boolean isAnalyzerMatched(Map<String,
String> properties, String a
buildAnalyzerIdentity(properties));
}
+ String resolvedAnalyzer = resolveAnalyzerName(normalizedAnalyzer);
String preferredAnalyzer =
InvertedIndexProperties.getPreferredAnalyzer(properties);
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
- return normalizedAnalyzer.equalsIgnoreCase(preferredAnalyzer);
+ return
resolvedAnalyzer.equals(resolveAnalyzerName(preferredAnalyzer));
}
String parser =
InvertedIndexProperties.getInvertedIndexParser(properties);
if (Strings.isNullOrEmpty(parser)) {
- return normalizedAnalyzer.equalsIgnoreCase("default")
- ||
normalizedAnalyzer.equalsIgnoreCase(INVERTED_INDEX_PARSER_NONE);
+ return resolvedAnalyzer.equals("default")
+ || resolvedAnalyzer.equals(INVERTED_INDEX_PARSER_NONE);
}
- return normalizedAnalyzer.equalsIgnoreCase(parser);
+ return resolvedAnalyzer.equals(parser.trim().toLowerCase(Locale.ROOT));
Review Comment:
Fixed in `2558085d88f6e815ff6775ce0c341a83fa1baef2`.
I reproduced both selection failures with native tests.
`InvertedIndexPropertiesTest.testExplicitBuiltinIkSelectsMatchingModeAndLowercase`
selected the first smart/lowercase-disabled index rather than the default
max-word index, and
`InvertedIndexIteratorTest.MatchBindsDistinctIkModesAndLowercaseStates`
collapsed the four physical configurations into one reader key. Both tests
failed before the fix and now pass.
FE matches an explicit built-in IK request to its effective default
configuration and serializes that selected index's existing mode, lowercase,
and outer character-filter fields. BE builds configuration-aware selection keys
from those fields while retaining the original provider name. The BE test uses
real MATCH predicate/configuration parsing and iterator selection, mocking only
the physical reader boundary. It distinguishes all four states; additional
guards cover outer filters and encoded-key collisions with legal policy names.
No Thrift schema or stored index format changed.
The normal SQL regression now includes the discriminating Chinese probe:
explicit built-in IK matches the `清华` term in `清华大学`, while implicit smart
selection does not. The uppercase probe is not claimed as proof of a
lowercase-disabled runtime fix; existing IK internal lowercasing behavior is
unchanged.
The official FE runner passed 118 tests; the related BE runner passed 178
tests, with 120 affected tests rerun successfully after the final source
corrections. All three selected SQL suites passed on the task-owned native ASAN
cluster.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -154,43 +203,71 @@ 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 (policy.isInvalid()) {
- return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ 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 "";
+ }
+ if (sortedProps.size() == 1) {
+ return normalizedType;
+ }
+ sortedProps.put(IndexPolicy.PROP_TYPE,
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(
Review Comment:
Fixed in `2558085d88f6e815ff6775ce0c341a83fa1baef2`.
The native identity test failed before the fix: a named `A -> a` character
filter added a distinct identity to an IK analyzer even though the tokenizer
absorbs that transformation. The native SQL CREATE reproduction also accepted
the duplicate instead of reporting the expected same-analyzer error.
Identity construction now resolves the actual tokenizer before
canonicalizing named character filters and passes IK's downstream lowercase
context into that step. It walks filters backward to determine whether a
transformation is absorbed, but preserves their original order in the completed
identity. After a meaningful intervening filter, earlier transformations are
not incorrectly removed. Unit coverage includes resolved tokenizer
aliases/shadowing and ordered filters.
The official FE runner passed 118 tests, including the complete identity
test class. The SQL regression covers both CREATE and ALTER duplicate rejection
for both `ik_smart` and `ik_max_word`; the two new analyzer suites and the
existing IK suite all passed against the task-owned native ASAN cluster.
--
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]
