airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4045723717
##########
be/src/storage/index/inverted/tokenizer/tokenizer.h:
##########
@@ -41,10 +45,50 @@ class DorisTokenizer : public Tokenizer, public
DorisTokenStream {
// Only use the parameterless reset method
void reset() override { _in = _in_pending; };
+ std::span<const int32_t> get_source_byte_offsets() const override {
+ return _source_byte_offsets_enabled ? std::span<const int32_t>
{_source_byte_offsets}
+ : std::span<const int32_t> {};
+ }
+
+ void set_source_byte_offsets_enabled(bool enabled) override {
+ _source_byte_offsets_enabled = enabled;
+ }
+
protected:
+ int32_t correct_source_offset(int32_t offset) const {
+ const auto* char_filter = dynamic_cast<const
DorisCharFilter*>(_in.get());
+ return char_filter == nullptr ? offset :
char_filter->correct_offset(offset);
+ }
+
+ void set_source_byte_offsets(std::string_view term, int32_t source_start) {
+ _source_byte_offsets.clear();
+ const auto* char_filter = dynamic_cast<const
DorisCharFilter*>(_in.get());
+ if (!_source_byte_offsets_enabled || char_filter == nullptr) {
Review Comment:
Fixed in 4119d0bf2e3. When provenance tracking is enabled, plain readers now
publish UTF-8 rune boundaries; character-filter correction is applied only when
a filter exists.
`PinyinFilterTest.TestWordDelimiterPreservesPlainTokenizerOffsetsAfterReset`
reproduced incorrect subterm offsets before the fix and passes afterward. It
covers both plain keyword and standard tokenizers, composed
word-delimiter/pinyin processing, and reset/reuse. The relevant BE ASAN test
selection passed (137 tests).
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -154,43 +205,60 @@ 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);
+ }
+ return sortedProps.toString();
Review Comment:
Fixed in 4119d0bf2e3. Named `char_replace` components now use the same
effective byte-set canonicalization as outer filters: pattern order and
duplicate bytes are ignored, self-replacements are removed, and an entirely
no-op filter contributes no identity.
`AnalyzerIdentityBuilderTest.testNamedCharReplaceIdentityUsesEffectiveByteSet`
and CREATE/ALTER duplicate-validation tests cover reordered, duplicate, and
no-op patterns. The native `test_analyzer_identity_semantics` regression suite
also passed those DDL paths.
##########
be/src/runtime/index_policy/index_policy_mgr.cpp:
##########
@@ -41,13 +41,72 @@ class SingleAnalyzerProvider final : public
segment_v2::inverted_index::Analyzer
const std::unordered_set<std::string> IndexPolicyMgr::BUILTIN_NORMALIZERS =
{"lowercase"};
-std::string IndexPolicyMgr::normalize_name(const std::string& name) {
+std::string IndexPolicyMgr::trim_name(const std::string& name) {
std::string result = name;
boost::algorithm::trim(result);
+ return result;
+}
+
+std::string IndexPolicyMgr::normalize_name(const std::string& name) {
+ std::string result = trim_name(name);
boost::algorithm::to_lower(result);
return result;
}
+const TIndexPolicy* IndexPolicyMgr::find_policy_by_name_locked(const
std::string& name) const {
+ const std::string exact_name = trim_name(name);
+ if (auto exact_it = _exact_name_to_id.find(exact_name); exact_it !=
_exact_name_to_id.end()) {
Review Comment:
Fixed in 4119d0bf2e3. FE resolves the saved policy spelling before MATCH
serialization and canonicalizes only built-ins. BE preserves that resolved
spelling for custom-provider dispatch and reader selection, so `IK`/`ik` and
case-distinct legacy policies no longer share a reader key. Legacy parser
properties remain case-insensitive.
Validation covers replayed policy selection, implicit/explicit MATCH Thrift
serialization, real Nereids MATCH translation, BE provider dispatch, and
selection between actual mock readers with colliding names. The BE
dispatch/reader tests failed before the fix and pass afterward; after
synchronizing master, the relevant selections passed 137 BE ASAN and 115 FE
tests. Both native analyzer regression suites passed, including ordinary
implicit MATCH, uppercase explicit custom aliases, and uppercase built-in IK.
This is component-level replay coverage plus current-version end-to-end
coverage, not a full mixed-version cluster upgrade test.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -45,14 +47,60 @@ public static String buildAnalyzerIdentity(
}
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
+ String builtinIkIdentity =
resolveBuiltinIkAnalyzerIdentity(properties, preferredAnalyzer);
+ if (builtinIkIdentity != null) {
+ return appendOuterCharFilterIdentity(builtinIkIdentity,
properties);
+ }
// For custom analyzer/normalizer, resolve to underlying config to
build identity
- return resolveAnalyzerIdentity(preferredAnalyzer,
defaultAnalyzerKey, log);
+ return appendOuterCharFilterIdentity(
+ resolveAnalyzerIdentity(preferredAnalyzer,
defaultAnalyzerKey, log), properties);
}
if (Strings.isNullOrEmpty(parser) ||
parserNone.equalsIgnoreCase(parser)) {
return defaultAnalyzerKey;
}
- return parser;
+ String legacyIkIdentity = resolveLegacyIkIdentity(properties, parser);
+ if (legacyIkIdentity != null) {
+ return appendOuterCharFilterIdentity(legacyIkIdentity, properties);
+ }
+ return appendOuterCharFilterIdentity(parser, properties);
+ }
+
+ private static String resolveBuiltinIkAnalyzerIdentity(
+ Map<String, String> properties, String analyzer) {
+ // BE defaults analyzer=ik to max-word mode. It has the built-in
ik_max_word base
+ // identity when no index-level tokenizer option changes its behavior;
the caller
+ // appends any outer char-filter identity separately.
+ if
(!InvertedIndexProperties.INVERTED_INDEX_PARSER_IK.equalsIgnoreCase(analyzer.trim()))
{
+ return null;
+ }
+ String lowerCase =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_LOWERCASE_KEY);
+ if (!Strings.isNullOrEmpty(lowerCase) &&
!Boolean.TRUE.toString().equalsIgnoreCase(lowerCase)) {
+ return null;
Review Comment:
Fixed in 4119d0bf2e3. Both IK identity paths now retain runtime mode and
disabled-lowercase state: analyzer-only IK defaults to max-word, while legacy
parser IK defaults to smart. Explicit legacy max-word remains equivalent to
analyzer-only max-word when their lowercase settings agree.
`AnalyzerIdentityBuilderTest.testDisabledLowercaseIkModesHaveDistinctIdentities`
and CREATE/ALTER duplicate-validation tests cover these comparisons. The
native `test_analyzer_identity_semantics` regression suite also passed the
accepted smart/max-word pair and the rejected equivalent max-word pair.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -45,14 +47,60 @@ public static String buildAnalyzerIdentity(
}
if (!Strings.isNullOrEmpty(preferredAnalyzer)) {
+ String builtinIkIdentity =
resolveBuiltinIkAnalyzerIdentity(properties, preferredAnalyzer);
Review Comment:
Fixed in 4119d0bf2e3. The synthetic built-in IK identity now applies only to
canonical lowercase `ik`; case-distinct legacy `IK` follows the exact saved
policy, consistent with BE dispatch.
`AnalyzerIdentityBuilderTest.testReplayedExactIkAnalyzerUsesCustomIdentity`
covers replayed `IK` and an equivalent custom standard analyzer. The identity
regression reproduced the collision before the fix and now passes;
corresponding CREATE/ALTER duplicate-validation tests also pass in the relevant
FE test selection.
--
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]
