Skip to content

feat(fts): add MeCab tokenizer for Japanese + tokenizer registry refactor - #57

Open
chiangchenghsin-hash wants to merge 112 commits into
LadybugDB:mainfrom
chiangchenghsin-hash:feat/fts-mecab-tokenizer-2
Open

feat(fts): add MeCab tokenizer for Japanese + tokenizer registry refactor#57
chiangchenghsin-hash wants to merge 112 commits into
LadybugDB:mainfrom
chiangchenghsin-hash:feat/fts-mecab-tokenizer-2

Conversation

@chiangchenghsin-hash

Copy link
Copy Markdown
Contributor

Summary

Adds a MeCab tokenizer for Japanese to the FTS extension, together with a small tokenizer registry refactor that makes adding new tokenizers a one-line registration, and a fix for a silent correctness bug in incremental index updates.

While working on this, I noticed the FTS tokenizer code referenced third_party/cppjieba but the directory was never vendored in this repo, so the Jieba (Chinese) tokenizer could not be built from source. This PR includes the vendored cppjieba tree (from the upstream cppjieba project, MIT license) so the extension builds self-contained.

Changes

New tokenizer architecture (fts/src/utils/tokenizer.{h,cpp}):

  • ITokenizer interface + TokenizerRegistry (factories registered by name) + TokenizerPool (instances cached per name+params and shared across index builds, incremental inserts and queries).
  • Indexing/query paths no longer construct a fresh Jieba instance (which loads a 14MB dictionary) on every call — this was a significant per-document and per-query cost.
  • FTSConfig.tokenizerParams (unordered map) replaces the hardcoded jiebaDictDir field; serialization is backward compatible (a magic marker distinguishes new catalogs from legacy ones, whose dict dir is folded into tokenizerParams["jieba_dict_dir"]).

MeCab (Japanese) tokenizer:

  • Vendored third_party/mecab/ (mecab 0.996, BSD-3) with the MSVC compatibility fixes needed for modern MSVC (missing WPATH_FORCE define, register keyword removal, std::binary_function, gated unsigned long long stream operator, static-link DLL_EXPORT handling).
  • The ipadic dictionary (54MB of EUC-JP CSV) is downloaded and compiled at configure/build time (mecab-dict-index -f euc-jp -t utf-8), so no large dictionary files are committed.
  • New tokenizer := 'mecab' option with mecab_dict_dir parameter; default dictionary is copied next to the built extension like the jieba dict.
  • Added fts_japanese.test covering basic queries, incremental insert, and a custom dict dir.

Bug fix — incremental insert used the wrong tokenizer:
createFTSIndexQuery rewrote the internal _CREATE_FTS_INDEX call without forwarding tokenizer/jieba_dict_dir, so the index's internal config silently fell back to 'simple'. Rows inserted after index creation were tokenized with whitespace splitting, so Chinese/Japanese terms were never indexed (queries could not match new rows). The parameters are now forwarded, with a regression test (ChineseIncrementalInsert in fts_chinese.test).

Why it matters

  • Japanese full-text search was previously impossible (no Japanese tokenizer).
  • The incremental-insert bug silently dropped new documents from the index for every non-simple tokenizer — affecting existing Chinese users today, not just the new Japanese path.
  • The registry/instance-pool refactor removes a per-insert/per-query dictionary reload that is the dominant cost of indexing CJK content.

Verification

Built with MSVC (Visual Studio 18 2026, Release) on Windows; all tests pass:

  • fts_chinese.test (Jieba) incl. new incremental-insert regression case
  • fts_japanese.test (MeCab) — basic, incremental insert, custom dict dir
  • fts_basic/error.test suites unaffected (tokenizer error message updated in sync)
  • Real-world smoke test: 652 paragraphs of a Japanese novel (松本清張《空の城》, ~580K chars) indexed in ~1.1s; queries for 東京/空の城/日本/戦争 return ranked BM25 hits, including rows inserted after index creation.

Note: I could not run the full .test suite in CI here (test targets are not enabled in my local build config), but all touched paths are covered by the smoke tests above.

Thank you for reviewing!

@adsharma

Copy link
Copy Markdown
Contributor

This PR includes the vendored cppjieba tree

Preference is to add third_party as a submodule if necessary instead of vendoring. We're already doing it for datasets submodule in various language bindings (e.g. ladybug-python).

Also why does this PR have 112 commits?

Given that FTS is a widely used module and the impact of such a large diff on the text size, it's probably best maintained as an out of tree extension.

Adding third_party/opengql was a mistake. It should be added to main repo's third_party and then added as a submodule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants