[feature](inverted-index) Add Japanese (Kuromoji) morphological analyzer - #64667
[feature](inverted-index) Add Japanese (Kuromoji) morphological analyzer#64667nishant94 wants to merge 25 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
@nishant94 have you tried icu analyzer? because I think icu could handle many different languages. |
@yiguolei The ICU Analyzer is not good as the Kuromoji. There is huge difference between icu and kuromoji when it comes to morphology of the Japanese words. So I think it worth it adding this new parser. |
|
Is the code under |
This is original code but it is modeled on Apache Lucene's kuromoji. |
FE UT Coverage ReportIncrement line coverage |
389fcfb to
b79db3c
Compare
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
db0ee69 to
06b4ef6
Compare
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…wn words - Implemented functionality in the Kuromoji Viterbi segmenter to decompose unknown (out-of-vocabulary) words into per-character unigrams when in extended mode, aligning with Lucene's JapaneseTokenizer behavior. - Added unit tests to validate the correct segmentation of unknown words in both normal and extended modes, ensuring expected outputs for various input scenarios.
- Modified error messages to include 'kuromoji' parser in the parser mode validation. - Enhanced tests for the Japanese analyzer to assert expected tokenization results.
- Introduced a new configuration option `enable_kuromoji_analyzer` to toggle the Kuromoji analyzer functionality. - Updated unit tests to validate the behavior of the Kuromoji analyzer when enabled and disabled. - Modified tests to enable the Kuromoji analyzer for specific test cases.
- Updated the namespace for Kuromoji components from `doris::segment_v2::kuromoji` to `doris::segment_v2::inverted_index::kuromoji` across multiple files for better organization and clarity.
- Updated the CMake configuration to ensure the required Kuromoji dictionary files are present at build time, failing the build if any are missing. - Modified the KuromojiAnalyzer and KuromojiTokenizer to throw exceptions when the dictionary is not loaded, preventing silent fallbacks to per-codepoint tokenization. - Improved error handling and validation in the dictionary loading process to ensure robust operation. - Updated unit tests to validate the new behavior, ensuring that missing dictionaries trigger appropriate errors.
- Replaced the `ending_at` vector with `end_head` and `end_next` for better memory management and performance during node processing. - Updated node addition and traversal logic to utilize the new data structures, enhancing the segmenter's efficiency in handling word segmentation.
- Changed the values in the `unk.per_category[CAT_DEFAULT]` entry from `{5, 5, 4769, "unk-default"}` to `{2, 2, 4769, "unk-default"}` to correct the test setup.
- Modified CMake configuration to conditionally include the Kuromoji dictionary files only for non-test builds (MAKE_TEST=ON). - Adjusted the custom target for generating the Kuromoji dictionary to reflect the new conditional behavior, ensuring it remains a manual target during unit-test builds.
- Added checks for empty trie and out-of-range category mappings in the Kuromoji dictionary. - Updated tests
- Added logic to return the Kuromoji search mode based on the analyzer property. - Updated unit tests accordingly.
- This enhancement ensures that the necessary Kuromoji dictionary source is available for builds, improving the setup process for users.
- Updated test cases in `test_japanese_analyzer.groovy` to use query-time (qt_) assertions for better readability and maintainability.
- Introduced a penalty calculation mechanism for segmenting words based on the analyzer mode, improving the accuracy of word segmentation. - Added caching for the current same-category run's byte end to optimize processing of grouped unknown words, reducing unnecessary rescans.
- Moved mecab-ipadic staging into a conditional block based on the BUILD_BE flag.
- Refactored the logic for setting the Kuromoji analyzer mode to ensure it defaults to the search mode when the parser mode is empty or set to coarse granularity.
- Introduced sorting of CSV file paths before processing to ensure consistent order. - Enhanced error handling during lexicon parsing, including detailed error messages for failed parses. - Added sorting of BuilderWord entries by cost and IDs.
- Implemented a new utility function to decode the first UTF-8 code point from a given string view, enhancing character processing capabilities. - Updated the KuromojiTokenizer to skip unknown tokens based on their character category, improving tokenization accuracy.
- Introduced a new executable `kuromoji_build_dict` to compile the UTF-8 mecab-ipadic source into binary files. - Added custom commands and targets to generate the Kuromoji dictionary, ensuring it is built as part of the overall project unless in test mode.
- Updated the CMake configuration to prepend the Kuromoji JVM library directory to the DYLD_LIBRARY_PATH and LD_LIBRARY_PATH environment variables during the dictionary generation process.
de67da3 to
8de3899
Compare
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: All Codex review accounts are usage-limited; earliest retry is 2026-08-08T03:32:00Z. Please trigger /review again after that time. |
|
/review |
|
Codex automated review failed and did not complete. Error: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated full-scope review of 8de3899cf47421e67ca2bfd2527c1bc9d94becdb. There was no additional user-provided focus, so I reviewed the entire supplied PR diff. I found five distinct new issues (three major, two minor) after duplicate-suppressing against all current review threads.
Critical checkpoint conclusions:
- Goal/end-to-end behavior: the PR coherently adds IPADIC generation/loading and routes Kuromoji through index writes, indexed and slow MATCH-family analysis, SEARCH construction, and TOKENIZE; ordinary long-term correctness is blocked by the term-clipping finding.
- Scope and API/storage/SQL compatibility: the broad build, runtime, FE, license, and test changes are feature-related. No new thrift or transaction schema is introduced; persisted analyzer strings and index segments remain subject to rolling-version issues already owned by live threads.
- Concurrency, initialization, and lifecycle: successful dictionaries are mutex-published, immutable, process-lived, and safely back analyzer raw pointers; token streams are not shared across query calls. Failed-load lifetime is defective because a null cache entry can never recover at the same path. No separate lock-order, static-initialization, callback, or ownership issue was found.
- Configuration semantics: the enable flag is mutable and checked on analyzer creation. Homogeneous current FE/BE defaults agree, but the regression destructively resets per-BE state. Readiness/config skew and analyzer-only validation gaps remain current but are already covered by live threads.
- Runtime/version compatibility: current FE/BE parser and mode behavior agrees. Old-FE/old-BE rolling behavior and a legacy custom analyzer named
kuromojiremain unsafe under existing owned discussions; the new distinct compatibility failure is use of a CMake 3.25-only command on the declared 3.19.2+ build path. - Parallel paths and control flow: scalar/array writers, indexed/slow MATCH, SEARCH, phrase-family construction, and TOKENIZE were traced to the same analyzer/mode. Loader errors fail visibly. Remaining position, punctuation, compound-emission, malformed-input, and corrupt-artifact cases are already owned by live threads.
- Tests and expected results: synthetic BE/FE tests are discovered, while real-dictionary UTs can skip and the new P0 output/format/phrase coverage gaps remain current under existing threads. The distinct additional isolation issue is failure to restore the cluster configuration. No builds or tests were run, as required by the review prompt.
- Observability: load failures log their directory and status and analyzer construction surfaces the error; no distinct missing metric/log issue was substantiated.
- Transactions, persistence, and data writes: no transaction/EditLog protocol changes were introduced. Current-version write/query analysis agrees, aside from deterministic long-term aliasing; distributed rolling/config-write hazards are already covered live-thread issues.
- FE/BE variables: parser, analyzer, and mode strings use the existing property/MatchPredicate paths with no missing send site found.
- Performance: offline generation is one-time and successful mmap reuse is read-only. Long-OOV CPU/lattice amplification is already owned by an existing thread; no additional performance issue survived review.
- Other correctness: packaging, thirdparty staging, deterministic generation, licenses, dictionary structure, UTF-8 stepping, Viterbi cost orientation, normalization, and all changed tests were rechecked. The five inline findings below are the complete non-duplicate set for this head.
| "${KUROMOJI_DICT_OUT}/chardef.bin" "${KUROMOJI_DICT_OUT}/unkdict.bin" | ||
| COMMAND ${CMAKE_COMMAND} -E make_directory "${KUROMOJI_DICT_OUT}" | ||
| COMMAND ${CMAKE_COMMAND} -E env | ||
| --modify "DYLD_LIBRARY_PATH=path_list_prepend:${KUROMOJI_LIBJVM_DIR}" |
There was a problem hiding this comment.
[Major] Keep this command compatible with the declared CMake minimum. cmake -E env --modify was added in CMake 3.25, while this project still accepts 3.19.2 and docker/compilation/Dockerfile.gcc10 pins 3.22.1. On those supported toolchains configuration succeeds, but the ALL kuromoji_dict edge fails before producing the four binaries because CMake does not recognize --modify. Use a 3.19-compatible environment wrapper/syntax, or raise and enforce the minimum before configuring.
| Status st = load(dir, &dict); | ||
| if (!st.ok()) { | ||
| LOG(WARNING) << "kuromoji: failed to load dictionary from " << dir << ": " << st; | ||
| cache.emplace(dir, nullptr); |
There was a problem hiding this comment.
[Major] Do not cache a failed dictionary load for the lifetime of the BE. If the first request observes a missing, corrupt, or temporarily unavailable file, this inserts nullptr for the directory; every later analyzer creation returns that cached failure even after all four valid artifacts are repaired or restored at the same path. The mutable feature gate makes first access after startup a supported lifecycle, but neither toggling it nor repairing the files can recover this BE. Cache successful immutable dictionaries only, or give negative entries a bounded retry/invalidation path, and cover fail-then-install-then-retry at one directory.
| std::string& token_text = tokens_text_[buffer_index_++]; | ||
| // reset() already segmented and normalized the terms; hand them out one at a | ||
| // time, capped at the CLucene maximum term length. | ||
| size_t size = std::min(token_text.size(), static_cast<size_t>(LUCENE_MAX_WORD_LEN)); |
There was a problem hiding this comment.
[Major] Do not silently truncate a Kuromoji morpheme to 255 bytes. Grouped OOV tokens can be up to 1,024 code points, so two terms with the same first 255 bytes but different suffixes become identical in both the index and query analyzer, creating false MATCH-family hits; a Japanese/supplementary token can also be cut in the middle of a UTF-8 code point and leak malformed output through TOKENIZE. Reject/drop over-limit terms under one explicit index/query policy, or split them on validated UTF-8 boundaries, and test differing long suffixes plus a multibyte boundary.
| assertTrue(tokenStr.contains('"token": "東京"')) | ||
| } finally { | ||
| sql "DROP TABLE IF EXISTS ${tableName}" | ||
| set_be_config("enable_kuromoji_analyzer", "false") |
There was a problem hiding this comment.
[Minor] Restore each backend's original enable_kuromoji_analyzer value instead of forcing false here. The setting is mutable and true is a valid pre-suite state (with different BEs potentially configured differently), so a successful or failed run currently changes the shared cluster for later suites. Snapshot the value per BE before enabling it, check each update result, and restore those values in finally.
| sh thirdparty/build-thirdparty.sh mecab_ipadic | ||
|
|
||
| # 2. the CMake target builds the offline compiler and produces the *.bin here | ||
| ninja -C be/ut_build_RELEASE kuromoji_dict |
There was a problem hiding this comment.
[Minor] Point this command at a build tree that actually defines kuromoji_dict. The named be/ut_build_RELEASE directory is configured by run-be-ut.sh with MAKE_TEST=ON, while be/CMakeLists.txt creates both Kuromoji generator targets only under if (NOT MAKE_TEST), so following these checked-in regeneration instructions fails with an unknown target. Document a normal MAKE_TEST=OFF build directory, or intentionally expose the target in the documented graph.
7 participants
What problem does this PR solve?
Issue Number: #64646
Related PR: None
Problem Summary:
Doris has no Japanese-aware tokenizer for the inverted index. Japanese text has no spaces between words, so the existing parsers can't segment it and
MATCH/MATCH_PHRASEon Japanese columns end up with poor recall and precision.This PR adds a built-in
kuromojiparser for Japanese, in the same style as the existing Chinese IK analyzer. It's opt-in per column:After indexing, MATCH, MATCH_PHRASE and TOKENIZE() run against the segmented Japanese terms.
How it works:
be/src/storage/index/inverted/analyzer/kuromoji/, so there's no JVM on the indexing path. KuromojiAnalyzer / KuromojiTokenizer mirror the IK analyzer/tokenizer, with a Viterbi cost-model segmenter over the IPADIC connection-cost matrix.Dictionary source is mecab-ipadic-2.7.0-20070801 (NAIST-2003 license, the same lexicon Lucene kuromoji uses).
Release note
Support Japanese text tokenization in the inverted index via a new kuromoji parser (
PROPERTIES("parser"="kuromoji")), withsearch/normal/extendedmodes.Check List (For Author)
parser="kuromoji".Check List (For Reviewer who merge this PR)