-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support FullText search on SQLite #723
Conversation
158a0cc
to
52f7e50
Compare
Locally, I am seeing these test failures. I think there is something race-y about table creation. The failures are not all reproducible if run in isolation.
|
Some of the above failures are unrelated, and simply due to this branch being old and carrying some upstream incompatibility issues that have been fixed on |
0b0ac2e
to
f799703
Compare
Unit tests pass locally for me now.
|
""" | ||
INSERT INTO metadata_fts5(rowid, metadata) | ||
SELECT id, metadata FROM nodes; | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing trialing comma after close-quotes.
For reference I wanted to drop this link here --- this is where I found the approach we settled on for creating a |
f1e8be8
to
7eeb68f
Compare
This PR has commits from @Kezzsim and me. It adds support for full text search backed by SQLite.
SQLite full text search support via FTS5 is solid and feature-ful, but the implementation is somewhat different. It employs a
VIRTUAL TABLE
instead of anINDEX
. Database triggers are used to keep the content of this table in sync with thenodes
table. We must perform aJOIN
on that virtual table in order to filter our results with aMATCH
query against the indexed nodemetadata
.We get queries that look like:
(as shown by setting
TILED_ECHO_SQL=1
when starting the server).I think the implementation is about as clean as it can be, until we go and refactor all the SQLAlchemy code into a separate module. But I'd like to kick the tires a bit more before we merge.
We also need to test the migration interactively, and particularly ensure that existing rows in the
nodes
table are added to theVIRTUAL TABLE
.Closes #546