github-actions[bot] commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4000927923
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -50,9 +52,32 @@ public static String buildAnalyzerIdentity(
if (Strings.isNullOrEmpty(parser) ||
parserNone.equalsIgnoreCase(parser)) {
return defaultAnalyzerKey;
}
+ String legacyIkIdentity = resolveLegacyIkIdentity(properties, parser);
+ if (legacyIkIdentity != null) {
+ return legacyIkIdentity;
+ }
return parser;
}
+ private static String resolveLegacyIkIdentity(Map<String, String>
properties, String parser) {
+ if
(!InvertedIndexProperties.INVERTED_INDEX_PARSER_IK.equalsIgnoreCase(parser)
+ || !Strings.isNullOrEmpty(properties.get(
+
InvertedIndexProperties.INVERTED_INDEX_PARSER_CHAR_FILTER_TYPE))) {
+ return null;
+ }
+
+ String mode =
properties.get(InvertedIndexProperties.INVERTED_INDEX_PARSER_MODE_KEY);
+ if (Strings.isNullOrEmpty(mode)) {
+ mode = InvertedIndexProperties.INVERTED_INDEX_PARSER_SMART;
+ }
+ String tokenizer = normalizeBuiltinComponentName(mode,
IndexPolicyTypeEnum.TOKENIZER);
+ if (!"ik_smart".equals(tokenizer) && !"ik_max_word".equals(tokenizer))
{
+ return null;
+ }
+ return buildIdentityFromPolicyProperties(
Review Comment:
[P1] Do not equate runtime-distinct legacy IK analyzers
This synthetic component is resolved with named-policy precedence and
without checking accepted legacy behavior. Two concrete cases false-collide:
`parser=ik, lower_case=false` emits `LIUDE` for full-width `ＬＩＵＤＥ`, while the
custom factory hardcodes lowercase and emits `liude`; and a replayed
pre-upgrade TOKENIZER policy named `ik_smart` (for example `type=standard`)
hijacks this synthetic identity even though legacy runtime bypasses policies
and still constructs `IKAnalyzer`. CREATE and ALTER then reject
runtime-distinct indexes as duplicates. Build the legacy identity from the
literal built-in IK mode, collapse only behavior-equivalent property sets, and
cover both cases.
##########
be/src/storage/index/inverted/token_filter/token_filter.h:
##########
@@ -28,9 +28,15 @@ class DorisTokenFilter : public TokenFilter, public
DorisTokenStream {
void reset() override { _in->reset(); }
+ std::span<const int32_t> get_source_byte_offsets(std::string_view term)
const override {
Review Comment:
[P1] Carry source offsets through transforming filters
This generic forwarding keys provenance only by the current term text, but a
transforming filter inherits it unchanged. With an analyzer
`tokenizer=ik_smart, token_filter=word_delimiter,pf`, where `pf` is the
FE-valid Pinyin policy `type=pinyin, ignore_pinyin_offset=false` with its
output defaults, `ＬＩＵＤＥ１２３` gives IK a map for normalized `liude123`;
word-delimiter emits `liude`, and forwarding that subterm makes IK return no
map. Pinyin then emits `liu` / `de` at normalized `[0,3)` / `[3,5)` instead of
original `[0,9)` / `[9,15)`. Please carry the map as token provenance and let
transforming filters slice/update it, or reject unsupported filter orderings.
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +66,55 @@ Token* IKTokenizer::next(Token* token) {
return nullptr;
}
- std::string& token_text = tokens_text_[buffer_index_++];
+ TokenData& token_data = tokens_[buffer_index_++];
// full-width to half-width, and lowercase
// TODO(ryan19929): do regularizeString in fillBuffer.
- CharacterUtil::regularizeString(token_text, this->lowercase);
- size_t size = std::min(token_text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
- token->setNoCopy(token_text.data(), 0, static_cast<int32_t>(size));
+ CharacterUtil::regularizeString(token_data.text, this->lowercase);
+ current_token_ = &token_data;
+ size_t size = std::min(token_data.text.size(),
static_cast<size_t>(LUCENE_MAX_WORD_LEN));
+ set(token, std::string_view(token_data.text.data(), size));
+ token->setStartOffset(token_data.start_offset);
+ token->setEndOffset(token_data.end_offset);
return token;
}
+std::span<const int32_t> IKTokenizer::get_source_byte_offsets(std::string_view
term) const {
+ if (current_token_ == nullptr || term != current_token_->text) {
+ return {};
+ }
+ return current_token_->source_byte_offsets;
+}
+
+void IKTokenizer::reset() {
+ if (_in_pending == nullptr) {
+ return;
+ }
+ inverted_index::DorisTokenizer::reset();
+ _in_pending.reset();
+ reset(_in.get());
+}
+
void IKTokenizer::reset(lucene::util::Reader* reader) {
+ _in_pending.reset();
this->input = reader;
this->buffer_index_ = 0;
this->data_length_ = 0;
- this->tokens_text_.clear();
+ this->tokens_.clear();
+ this->current_token_ = nullptr;
try {
buffer_.reserve(input->size());
ik_segmenter_->reset(reader);
Lexeme lexeme;
while (ik_segmenter_->next(lexeme)) {
- tokens_text_.emplace_back(lexeme.getText());
+ TokenData token_data {
+ .text = lexeme.getText(),
+ .start_offset =
static_cast<int32_t>(lexeme.getByteBeginPosition()),
+ .end_offset =
static_cast<int32_t>(lexeme.getByteEndPosition()),
+ .source_byte_offsets = {}};
+ token_data.source_byte_offsets =
Review Comment:
[P2] Avoid source-map work on streams that cannot consume it
Every emitted IK term now runs `build_source_byte_offsets()`; a
no-width-change term gets a complete detector pass and is then scanned/copied
again by `regularizeString()` in `next()`. Repository-wide, Pinyin is the only
consumer, yet existing legacy IK analysis uses this tokenizer too. In addition,
every eagerly buffered token now retains two offsets and a vector object, while
width-changing terms retain full boundary arrays until reset. Thus ordinary
no-Pinyin indexing/query analysis pays input-linear CPU and peak-memory growth.
Please fuse mapping with the existing regularization pass and make storage
opt-in/lazy for a chain that can actually consume it.
##########
be/src/storage/index/inverted/tokenizer/ik/ik_tokenizer_factory.h:
##########
@@ -0,0 +1,49 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "common/config.h"
+#include "storage/index/inverted/analyzer/ik/IKTokenizer.h"
+#include "storage/index/inverted/analyzer/ik/dic/Dictionary.h"
+#include "storage/index/inverted/tokenizer/tokenizer_factory.h"
+
+namespace doris::segment_v2::inverted_index {
+
+class IKTokenizerFactory : public TokenizerFactory {
+public:
+ explicit IKTokenizerFactory(bool use_smart) : _use_smart(use_smart) {}
+ ~IKTokenizerFactory() override = default;
+
+ void initialize(const Settings& settings) override {}
+
+ TokenizerPtr create() override {
+ auto ik_config = std::make_shared<Configuration>(_use_smart, true);
+ ik_config->setDictPath(config::inverted_index_dict_path + "/ik");
+ Dictionary::initial(*ik_config);
Review Comment:
[P1] Convert failures from lazy IK creation at the real callers
This performs the first dictionary load only when `CustomAnalyzer` creates a
stream. `FunctionTokenize`'s `CLuceneError` catch has already ended before
`_do_tokenize()` / `get_analyse_result()` reaches this call, and row-fallback
MATCH plus analyzed SEARCH likewise have only Status/Doris-exception boundaries
afterward. When the configured IK dictionary is missing, unreadable, or corrupt
and the first IK use is a named custom analyzer, the raw `CLuceneError`
therefore escapes instead of becoming an analyzer error. Please convert it at
every actual lazy tokenization boundary, or put creation behind one
Result/Status-producing abstraction, and cover this first-use failure.
--
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]
