Skip to content
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

Full text search (fts5) #1163

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Full text search (fts5) #1163

wants to merge 17 commits into from

Conversation

nmeibergen
Copy link

I simply took the changes made in the previous pull request #984, bumped up to fts5, and merged these with the current master.

@radex What's left in order to get this through in your opinion?

PS. I'm currently experimenting with this functionality in my application, first results seem good, in line with expectation.

@radex
Copy link
Collaborator

radex commented Sep 13, 2021

@nmeibergen Can you go through comments in previous PR? There were some unresolved issues that were waiting for corrections before I could merge.

As for bumping to fts5… I'm not comfortable with that -- IIRC there are some non-trivial changes between fts5 and previous versions, it's not merely a straight-forward upgrade. So it would be better if the FTS settings (version, tokenizer, etc.) were configurable somehow in the schema

@radex
Copy link
Collaborator

radex commented Sep 22, 2021

@nmeibergen hey, are you interested in finishing that?

@nmeibergen
Copy link
Author

@radex I am definitely interested in finishing up this request. My life is just a little upside down due to the coming of a baby, so some time from my side will be needed before I'll continue.

With regards to versions and settings of FTS, I agree that that would be the best solution. I'll look into that.

@radex radex mentioned this pull request Feb 4, 2023
6 tasks
@radex
Copy link
Collaborator

radex commented Feb 4, 2023

@nmeibergen hey, are you maybe still interested in finishing this? ;)

@nmeibergen
Copy link
Author

@radex I am interested, but I don't see it happening on the short term. There is just too much stuff going on for me to work on currently. But if at some point there is a good gap of time I will be happy to finish this up though!

@radex
Copy link
Collaborator

radex commented Feb 7, 2023

No worries, thanks for the update!

@ororsatti
Copy link

@radex I'd like to help closing this PR.
I do have couple of issues with it currently,

  1. it seems the author of this PR (@nmeibergen ) said there is some sort of issue with the triggers, and i hope you'd be able to read it and answer him.
  2. The sheer size of the db with fts. We currently create a copy of the required columns, i.e if I have table 'Posts' which contains id,title,body , and I want to be able to add fts on body, with this current implementation I need to store the body column twice. I believe the fix here is to create the fts coulmns once on the fts table and when ever querying a model with fts on it, we just select from two tables in order to build the model on the js side. I need to invest more time thinking about how should migrations work on this scenarios, since we would have to create the new fts table, copy all fts columns' data to it, and remove those columns from the original table.
  3. Configuring the fts, I dont see a reason to use a different fts version, as my research on this topic, fts5 is the most efficient on large datasets, but I would like to hear from you about the reasons to allow fts3-4. On any how, while I implemented that (basically copied this PR and added some things) I was able to add config object to the fts.
  4. Migrations seem to not be mentioned here at all. How should we handle that? I was thinking creating the fts table, copy the needed columns and deleting them from the original content, but im not sure how will it preform on larger datasets, so I would like to get your insights on it.

Also, couple more questions,
I'm working on a different PR about adding SQLCipher (you can check it out here) but when ever I run ios/android tests it seems to fail, do I miss any steps? do I need to run some example project for them to work? not really sure.
Also, kinda new to this all OS, how should I rebase\fork this PR to me and push to this PR?

@samducker
Copy link

@radex @ororsatti is this still a goal of the project to have FTS search built in to watermelondb?

@ororsatti
Copy link

@radex @ororsatti is this still a goal of the project to have FTS search built in to watermelondb?

I hope so.

@radex
Copy link
Collaborator

radex commented Nov 22, 2024

It is a goal, yes -- unfortunately I haven't had the funding (or free time) to work on it :( And while many contributors did amazing job pushing the branch forward, the PRs always needed a little bit more work to be mergeable

@ororsatti
Copy link

It is a goal, yes -- unfortunately I haven't had the funding (or free time) to work on it :( And while many contributors did amazing job pushing the branch forward, the PRs always needed a little bit more work to be mergeable

Maybe we can discuss it? what are the goals regarding this? they seem to not be clear to me... I have been watching this lib for over a year now, i even have a open PR, but no response from you. I enjoyed working on this lib, maybe you can let us know what are the goals, id be happy to contribute.

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.

7 participants