Fix extension initialization: bind the address of the fts5_api out-pointer, and accept iVersion >= 2 - #37
Open
Saeris wants to merge 1 commit into
Open
Conversation
…ion >= 2 `lindera_fts5_tokenizer_init` could never load: two bugs sit on the init path, and the second is unreachable until the first is fixed. `bind_fts5_pointer` passed the pointer's VALUE rather than its address. `target` is `&mut *mut FTS5API`, so `target.cast()` auto-derefs the reference and hands `sqlite3_bind_pointer` the inner (null) pointer instead of `&pRet` as SQLite's documented `fts5_api_from_db` requires. Nothing reports an error — bind returns SQLITE_OK and step returns SQLITE_ROW — because nothing is malformed; fts5() simply has nowhere to write, so `p_fts5_api` stays null and init fails at SQLITE_INTERNAL. `ensure_fts5_api_version` then required exactly 2. SQLite 3.47.0 raised `fts5_api.iVersion` to 3 (adding fts5_tokenizer_v2), and the change is purely additive: `xCreateTokenizer`, the only entry point used here, is unchanged and keeps its position. An equality test rejects every SQLite released since, so it now accepts any version at or above the minimum. Verified against SQLite 3.53.0 with --features=embed-ipadic: the extension loads and all five CJK probes match (日本語, 辞書, 図書館, 食べる, コーヒー) — none of which the stock unicode61 tokenizer finds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #34.
The extension could not be loaded at all in the LTS version of
node:sqlitebecauselindera_fts5_tokenizer_initalways returned an error. There are two bugs on that path, and both have to be fixed for the extension to initialize; the second is only reachable once the first is fixed.1.
bind_fts5_pointerpassed the pointer's value, not its addressSQLite's documented way to obtain the FTS5 API is
fts5_api_from_db, from FTS5 §7 Extending FTS5:Note
&pRet— fts5() writes the api pointer into that variable.targethere is&mut *mut FTS5API, sotarget.cast::<c_void>()auto-derefs the reference and passes the inner*mut FTS5API(i.e.pRet's own value, which is null at that moment) instead of&pRet.sqlite3_bind_pointerreturnsSQLITE_OKandsqlite3_stepreturnsSQLITE_ROW, because nothing is technically wrong; fts5() simply has nowhere to write.p_fts5_apistays null and init fails atok_or(SQLITE_INTERNAL).Instrumented, before the fix:
The fix binds
(&raw mut *target), matching the C pattern.2.
ensure_fts5_api_versionrequired exactly 2Once the pointer arrives, init fails at the next gate. SQLite 3.47.0 (2024-10-21) raised
fts5_api.iVersionfrom 2 to 3, addingfts5_tokenizer_v2withxCreateTokenizer_v2/xFindTokenizer_v2(3.47.0 changelog).ext/fts5/fts5.hnow reads:The bump is purely additive:
xCreateTokenizer(the only entry point this extension uses) is unchanged and keeps its position in the struct, with the v2 members appended afterxCreateFunction. So,, an exact-match test rejects every SQLite released in the last ~21 months for no reason. Changed to>=, which still rejects a genuinely older API that would be missing members we rely on.Verification
Built
--release --features=embed-ipadicand loaded into SQLite 3.53.0 via Node's built-innode:sqlite:Every one of those scores 0 with the stock
unicode61tokenizer, so this exercises real Lindera segmentation end to end.cargo test --release --features=embed-ipadicpasses (2 tests + 1 doctest), andcargo fmt --checkis clean.cargo clippyreports one pre-existingcollapsible_ifwarning inDrop for PreparedStatement, untouched by this change.Re-verified after rebasing onto
mainat #36 (lindera 2.0.1 → 5.0.1): builds clean, same 5/5, tests andfmt --checkstill pass.