Details

Reviewers

tomek
kamil

Commits

rCOMMb6f1abd14fe2: [native](web) Add message_search fts5 table

Summary

issue: ENG-8640
By default all columns of fts5 table are indexed for fulltext search. We want to avoid doing this for original_message_id and message_id which represent message ids. That's why we mark them as UNINDEXED.
From docs:
"It is an error to add types, constraints or PRIMARY KEY declarations to a CREATE VIRTUAL TABLE statement used to create an FTS5 table. "

Test Plan

Tested that the migration and new db setup don't throw

Diff Detail

Repository

rCOMM Comm

Branch

inka/search

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Jul 8 2024, 7:15 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJul 8 2024, 7:15 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 8 2024, 7:31 AM

Harbormaster failed remote builds in B30200: Diff 42112!

Add porter tokenizer, build wasm

inka added inline comments.Jul 15 2024, 4:21 AM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
856	We want to use the porter tokenizer to have messages be tokenized and stemmed using the porter algorithm. We use porter on the keyserver as well. Please see the docs for more details

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 15 2024, 4:50 AM

Harbormaster failed remote builds in B30323: Diff 42284!

Harbormaster completed remote builds in B30323: Diff 42284.Jul 15 2024, 8:08 AM

inka requested review of this revision.Jul 15 2024, 8:08 AM

tomek accepted this revision.Jul 16 2024, 3:20 AM

This revision is now accepted and ready to land.Jul 16 2024, 3:20 AM

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one. Sorry about that!

inka added a child revision: D12778: [native] Add SQLiteQueryExecutor code for inserting into the search table.Jul 17 2024, 3:25 AM

inka added a parent revision: D12693: [native][web] Add fts5 extension to SQLite.

Rebase

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one.

Thank you for letting me know!

Harbormaster completed remote builds in B30458: Diff 42457.Jul 18 2024, 5:11 AM

Could you update the spacing to match the rest of the file?

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
893 ↗	(On Diff #42457)	We have table definitions at the top, and below we define indexes, could you move this above `media_idx_container`?

Could you update the spacing to match the rest of the file?

Please make sure to avoid tab characters! I noticed a bunch of them when I landed migration 48, so I spent some time fixing them up

Rebase

Harbormaster completed remote builds in B30554: Diff 42601.Jul 22 2024, 12:16 AM

Move code

Harbormaster failed remote builds in B30566: Diff 42617!Jul 22 2024, 8:21 AM

ashoat added inline comments.Jul 22 2024, 1:52 PM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
739–744 ↗	(On Diff #42617)	Are you introducing tab characters or three spaces here? Let's do two spaces
904–909 ↗	(On Diff #42617)	Are you introducing tab characters or three spaces here? Let's do two spaces