Page MenuHomePhabricator

[CommCoreModule] add table for message store threads
ClosedPublic

Authored by kamil on Apr 17 2023, 9:54 AM.
Tags
None
Referenced Files
F3390080: D7468.id25482.diff
Fri, Nov 29, 10:02 PM
F3389957: D7468.diff
Fri, Nov 29, 9:17 PM
Unknown Object (File)
Tue, Nov 26, 2:26 PM
Unknown Object (File)
Tue, Nov 26, 2:23 PM
Unknown Object (File)
Thu, Nov 14, 8:09 PM
Unknown Object (File)
Fri, Nov 8, 2:47 PM
Unknown Object (File)
Fri, Nov 8, 2:47 PM
Unknown Object (File)
Fri, Nov 8, 2:47 PM
Subscribers

Details

Summary

This code introduces new table for threads part of messages store. Skipping messageIDs as it can be computed based on message table.

start_reached will be represented as integer 1 and 0 (but will be sent as string from js code according to the current convention)

last_navigated_to and last_pruned are typed as BIGINT, SQLite`s concept of "type affinity" allows us to use INTEGER here but I believe BIGINT might make it clear to someone reading the code that this can not be stored in int. There are both approaches in the codebase so I chose the one I think is better

I think we will be querying based only on id so an additional index will not be needed.

Test Plan

Tested migration for existing clients:

image.png (89×875 px, 54 KB)

image.png (299×917 px, 52 KB)

Tested creating a new database for fresh install:

image.png (82×614 px, 32 KB)

image.png (228×889 px, 41 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
kamil published this revision for review.Apr 18 2023, 2:50 AM
kamil added inline comments.
native/cpp/CommonCpp/DatabaseManagers/DatabaseQueryExecutor.h
47 ↗(On Diff #25224)

whoops, typo, should be replaceMessageStoreThreads, I will fix this shortly

tomek added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1088–1090 ↗(On Diff #25224)

Wondering if this can be replaced by a single db operation. If not, should we wrap it with transaction?

This revision is now accepted and ready to land.Apr 20 2023, 4:51 AM

fix typo

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1088–1090 ↗(On Diff #25224)

Wondering if this can be replaced by a single db operation.

Yeah, didn't find anything suitable for this :/

If not, should we wrap it with transaction?

I think the design is that all ops are wrapped in the transaction (see here)