github-actions[bot] commented on code in PR #64667: URL: https://github.com/apache/doris/pull/64667#discussion_r3757694778
########## be/src/storage/index/inverted/analyzer/kuromoji/KuromojiTokenizer.cpp: ########## @@ -0,0 +1,164 @@ +// 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. + +#include "storage/index/inverted/analyzer/kuromoji/KuromojiTokenizer.h" + +#include <algorithm> +#include <string_view> + +#include "common/exception.h" +#include "storage/index/inverted/analyzer/kuromoji/kuromoji_normalize.h" + +namespace doris::segment_v2 { + +namespace { +// Returns the idx-th comma-separated field of an IPADIC feature string +// (0=POS1 ... 6=base form, 7=reading, 8=pronunciation), or empty. +std::string_view feature_field(std::string_view feat, int idx) { + int cur = 0; + std::size_t start = 0; + for (std::size_t i = 0; i <= feat.size(); ++i) { + if (i == feat.size() || feat[i] == ',') { + if (cur == idx) { + return feat.substr(start, i - start); + } + ++cur; + start = i + 1; + } + } + return {}; +} + +// Part-of-speech (POS1) classes dropped for full-text search. A coarse subset of +// Lucene/OpenSearch's ja stoptags: particles, auxiliary verbs, conjunctions, +// symbols, fillers. (Full stoptags fidelity is a later refinement.) +bool is_stop_pos(std::string_view pos1) { + return pos1 == "\xE5\x8A\xA9\xE8\xA9\x9E" || // 助詞 (particle) + pos1 == "\xE5\x8A\xA9\xE5\x8B\x95\xE8\xA9\x9E" || // 助動詞 (auxiliary verb) + pos1 == "\xE6\x8E\xA5\xE7\xB6\x9A\xE8\xA9\x9E" || // 接続詞 (conjunction) + pos1 == "\xE8\xA8\x98\xE5\x8F\xB7" || // 記号 (symbol) + pos1 == "\xE3\x83\x95\xE3\x82\xA3\xE3\x83\xA9\xE3\x83\xBC"; // フィラー (filler) +} + +void ascii_lower(std::string& s) { Review Comment: [Major] Apply the full Unicode lower-case contract here. When lower_case=true, writer, indexed/slow MATCH, and TOKENIZE all reach this tokenizer, but ascii_lower() changes only A-Z. For example, an indexed ΜΈΓΑ remains uppercase while the query μέγα emits a different term, even though Doris's shared LowerCaseFilter and its tests define Unicode-aware folding (ÜBER ΜΈΓΑ -> über μέγα). Reuse the existing ICU-based implementation (while preserving lower_case=false), and add non-ASCII TOKENIZE plus persisted index/query coverage. ########## be/CMakeLists.txt: ########## @@ -976,6 +996,49 @@ if (BUILD_META_TOOL OR BUILD_INDEX_TOOL) add_subdirectory(${SRC_DIR}/tools) endif() +if (NOT MAKE_TEST) + # Offline generator: compiles the UTF-8 mecab-ipadic source into binary files. + add_executable(kuromoji_build_dict EXCLUDE_FROM_ALL ${SRC_DIR}/tools/kuromoji_build_dict.cpp) + target_include_directories(kuromoji_build_dict PRIVATE ${PROJECT_SOURCE_DIR}/..) + pch_reuse(kuromoji_build_dict) + set_target_properties(kuromoji_build_dict PROPERTIES ENABLE_EXPORTS 1) + if (COMPILER_CLANG) + target_compile_options(kuromoji_build_dict PRIVATE + -Wno-implicit-int-conversion + -Wno-shorten-64-to-32) + endif() + target_link_libraries(kuromoji_build_dict ${DORIS_LINK_LIBS}) + + set(KUROMOJI_IPADIC_SRC "${THIRDPARTY_DIR}/share/mecab-ipadic-2.7.0-20250920" + CACHE PATH "UTF-8 mecab-ipadic source directory used to generate the kuromoji dictionary") + set(KUROMOJI_DICT_OUT "${BASE_DIR}/dict/kuromoji") Review Comment: [Major] Keep generated dictionary outputs scoped to the current build tree. KUROMOJI_IPADIC_SRC is a per-tree cache variable and the README documents overriding it, but KUROMOJI_DICT_OUT is this shared source-tree directory and the install rule copies it. If tree B successfully generates from a custom source, its newer files can make tree A's default-source edge look up to date, so A silently packages B's dictionary; concurrent trees also race the same temporary paths. Generate and install from a build/config-specific directory with a provenance/completion stamp, and cover two build trees configured with different sources. ########## be/src/storage/index/inverted/analyzer/kuromoji/kuromoji_viterbi.cpp: ########## @@ -0,0 +1,291 @@ +// 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. + +#include "storage/index/inverted/analyzer/kuromoji/kuromoji_viterbi.h" + +#include <algorithm> +#include <cstddef> +#include <limits> + +namespace doris::segment_v2::inverted_index::kuromoji { + +namespace { + +constexpr int64_t KMJ_INF = std::numeric_limits<int64_t>::max() / 4; +constexpr uint32_t MAX_UNKNOWN_GROUP_CHARS = 1024; + +// Search/Extended-mode compound-decomposition penalties, matching Lucene's +// JapaneseTokenizer. Lengths are counted in code points. A token longer than the +// length threshold is penalized so the minimum-cost path prefers its shorter +// parts: all-kanji runs over KANJI_LENGTH chars, other runs over OTHER_LENGTH. +constexpr uint32_t SEARCH_MODE_KANJI_LENGTH = 2; +constexpr int64_t SEARCH_MODE_KANJI_PENALTY = 3000; +constexpr uint32_t SEARCH_MODE_OTHER_LENGTH = 7; +constexpr int64_t SEARCH_MODE_OTHER_PENALTY = 1700; + +struct DecodedCp { + char32_t cp; + uint32_t len; +}; + +// Decode one UTF-8 code point at text[pos]. Invalid/truncated -> single byte. +DecodedCp decode_utf8(std::string_view text, std::size_t pos) { + auto b0 = static_cast<unsigned char>(text[pos]); Review Comment: [Major] Honor the documented one-byte fallback for malformed UTF-8. For bytes C3 41, this branch only checks the lead byte and remaining length, treats 41 as a continuation, and consumes the valid A inside one two-byte morpheme. The tokenizer later copies C3 41 and lowercases it to invalid C3 61, so TOKENIZE can return malformed bytes and indexing loses the separately searchable A. Doris STRING values can carry such input via unhex. Use one strict shared decoder for Viterbi/category/width normalization (validating continuations, shortest form, surrogates, and U+10FFFF), choose an explicit invalid-input policy, and test short mixed-invalid inputs end to end. -- 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]
