airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4000333254
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +31,36 @@ 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);
+ 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);
Review Comment:
Fixed in 33dd8200870. PinyinFilter now maps token-local rune spans to
document-relative UTF-8 byte spans and preserves the full input span for
whole-token alternatives.
PinyinFilterTest.TestIKSmartOffsetsRemainDocumentRelative asserts all 12 terms
and exact offsets from 0 through 27 bytes.
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +31,36 @@ 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);
+ 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;
}
+void IKTokenizer::reset() {
Review Comment:
Fixed in 33dd8200870. IKTokenizer::reset() now consumes only a pending
shared reader; the legacy eager raw-reader path remains intact and a second
custom reset is a no-op. TestLegacyAndCustomResetContracts covers both direct
paths, and TestLegacyAndCustomArrayIndexWriterResetContracts exercises actual
CLucene IndexWriter indexing for legacy IK plus two custom ARRAY-like fields
and verifies indexed terms.
##########
be/src/storage/index/inverted/analyzer/ik/IKTokenizer.cpp:
##########
@@ -31,27 +31,36 @@ 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);
+ 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;
}
+void IKTokenizer::reset() {
+ inverted_index::DorisTokenizer::reset();
+ reset(_in.get());
+}
+
void IKTokenizer::reset(lucene::util::Reader* reader) {
this->input = reader;
this->buffer_index_ = 0;
this->data_length_ = 0;
- this->tokens_text_.clear();
+ this->tokens_.clear();
try {
buffer_.reserve(input->size());
ik_segmenter_->reset(reader);
Lexeme lexeme;
while (ik_segmenter_->next(lexeme)) {
- tokens_text_.emplace_back(lexeme.getText());
+ tokens_.push_back({lexeme.getText(),
+
static_cast<int32_t>(lexeme.getByteBeginPosition()),
Review Comment:
Fixed in 33dd8200870. AnalyzeContext::markBufferOffset() now advances by
getNextBytePosition(), matching the consumed boundary used by refill.
TestOffsetsAcrossBufferRefill checks exact byte offsets for 2,000 multibyte
tokens across repeated refills.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java:
##########
@@ -59,7 +59,8 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {
public static final String PROP_TOKEN_FILTER = "token_filter";
public static final String PROP_CHAR_FILTER = "char_filter";
public static final Set<String> BUILTIN_TOKENIZERS = ImmutableSet.of(
- "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin");
+ "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin",
+ "ik_smart", "ik_max_word");
Review Comment:
Fixed in 33dd8200870. FE validation now accepts ik_smart and ik_max_word
through type-only NoOperationValidator handling. PolicyValidatorTests covers
named TOKENIZER policies for both modes.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java:
##########
@@ -59,7 +59,8 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {
public static final String PROP_TOKEN_FILTER = "token_filter";
public static final String PROP_CHAR_FILTER = "char_filter";
public static final Set<String> BUILTIN_TOKENIZERS = ImmutableSet.of(
- "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin");
+ "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin",
+ "ik_smart", "ik_max_word");
Review Comment:
Fixed in 33dd8200870. BE now passes normalized built-in tokenizer and filter
names to factories, and AnalyzerIdentityBuilder canonicalizes built-in
tokenizer identities.
IndexPolicyMgrTest.BuiltinTokenizerNamesAreCaseInsensitive verifies uppercase
IK_SMART through analysis, with FE identity coverage as well.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java:
##########
@@ -59,7 +59,8 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {
public static final String PROP_TOKEN_FILTER = "token_filter";
public static final String PROP_CHAR_FILTER = "char_filter";
public static final Set<String> BUILTIN_TOKENIZERS = ImmutableSet.of(
- "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin");
+ "empty", "ngram", "edge_ngram", "keyword", "standard",
"char_group", "basic", "icu", "pinyin",
+ "ik_smart", "ik_max_word");
Review Comment:
Fixed in 33dd8200870. Existing named policies now take precedence over
built-ins consistently in FE validation, FE analyzer identity, and BE
resolution. Replay coverage verifies FE behavior, while
ExistingPolicyTakesPrecedenceOverNewBuiltinName confirms a legacy named
ik_smart ngram tokenizer still produces ab, bc, and cd.
##########
regression-test/suites/inverted_index_p0/analyzer/test_ik_custom_analyzer.groovy:
##########
@@ -0,0 +1,123 @@
+// 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.
+
+suite("test_ik_custom_analyzer", "p0") {
+ def pinyinFilter = "test_ik_pinyin_filter"
+ def smartAnalyzer = "test_ik_smart_pinyin_analyzer"
+ def maxWordAnalyzer = "test_ik_max_word_pinyin_analyzer"
+ def tableName = "test_ik_custom_analyzer"
Review Comment:
Fixed in 33dd8200870. The suite now uses a hardcoded sole table name,
qt_/order_qt_ queries, and a runner-generated golden. I generated the output
with the repository runner and reran the suite normally against an isolated
local FE/BE; both runs passed.
--
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]
