github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4025167521
##########
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:
[P1] Publish identity provenance for plain tokenizer inputs
This branch also returns when tracking is enabled but `_in` is an ordinary
reader. In the valid `keyword -> word_delimiter ->
pinyin(ignore_pinyin_offset=false)` chain over `liu-de`, Pinyin's opt-in
reaches Keyword, but Keyword publishes an empty map. WordDelimiter therefore
gives both generated `liu` and `de` the upstream `[0,6)` span, and Pinyin
preserves those whole-token spans instead of `[0,3)` and `[4,6)`. Standard case
splitting has the same path. This is distinct from the earlier character-filter
Keyword/Standard and IK-to-WordDelimiter threads because those sources already
produce a map. Please build raw UTF-8 rune boundaries whenever tracking is
enabled, applying `correct_offset()` only when a character filter exists, and
cover reset/reuse for both plain tokenizers.
##########
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:
[P1] Resolve exact custom analyzers before the built-in IK shortcut
A pre-upgrade Turkish-locale FE could persist an exact ANALYZER policy named
`IK` because the old default-locale normalization produced dotless `ı`. Current
property canonicalization preserves that exact spelling, and BE's
case-sensitive dispatch executes the saved custom policy. This
`equalsIgnoreCase` shortcut nevertheless assigns built-in max-word identity
before consulting the exact policy. For example, `IK={tokenizer=standard}` is
then considered different from an equivalent custom standard analyzer and equal
to runtime-distinct built-in max-word IK, corrupting both CREATE and ALTER
duplicate checks. This is distinct from the prior lifecycle thread because
exact runtime binding now succeeds; this semantic consumer bypasses it. Please
resolve an exact policy first and use the synthetic built-in identity only when
no exact binding exists, with replay coverage for the historical `IK` name.
##########
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:
[P1] Canonicalize named `char_replace` policies here too
This raw map serialization bypasses the new byte-set canonicalizer used for
index-level outer filters. Two valid named filters such as
`cf_ab={type=char_replace,pattern=ab,replacement=x}` and
`cf_ba={type=char_replace,pattern=ba,replacement=x}` therefore give otherwise
identical analyzers different identities. BE loads both patterns into the same
`bitset<256>`, so order and duplicates are discarded and the runtime streams
are identical; replacement-byte entries are no-ops as well. CREATE and ALTER
can consequently admit duplicate indexes. This is a separate named-component
expansion path from the existing outer-filter thread. Please apply the same
effective byte-set rules when resolving named CHAR_FILTER policies and cover
reordered, duplicate, and no-op patterns in both duplicate consumers.
##########
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:
[P1] Keep disabled-lowercase IK modes distinct
When this returns null, `{analyzer=ik,lower_case=false}` falls through
`resolveAnalyzerIdentity("ik")` and gets the literal identity `ik`. The sibling
legacy guard does the same for `{parser=ik,lower_case=false}`, so line 66 also
returns `ik`. Their runtime defaults differ: analyzer-only IK falls back to
coarse/max-word mode, while legacy parser IK defaults to smart mode. These
valid configurations can emit different terms, yet both CREATE and ALTER reject
them as duplicates. This cross-branch collision is distinct from the prior
legacy-versus-custom lower-case thread and the enabled-lowercase analyzer
default fix. Please retain both runtime mode and lowercase state in the
fallback identity and test smart/max-word comparisons in both duplicate 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:
[P1] Preserve the exact analyzer binding through MATCH dispatch
With a replayed exact `IK={tokenizer=keyword}` policy, current index
properties retain `IK`; both CLucene and SNII writers use case-sensitive
custom-analyzer dispatch and reach this exact map, so `abc def` is indexed as
one keyword term. MATCH takes a different path: FE `AnalyzerSelector`
lowercases the implicit index analyzer (and explicit `USING ANALYZER IK`), then
BE `AnalyzerConfigParser` classifies `ik` as the built-in max-word analyzer.
The normalized reader key still selects the custom-written index, so query and
index tokenization diverge and can return false negatives. This is distinct
from the FE duplicate-identity issue and the earlier validation/DROP lifecycle
thread. Please carry an exact analyzer spelling or resolved policy ID through
query analysis and use a collision-safe reader-selection identity, with
implicit and explicit MATCH upgrade tests.
--
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]
