Add integrity store ops for sqlite migration
Details
- Reviewers
kamil tomek atul - Commits
- rCOMM99e116bb3826: [lib] Add integrity store ops
flow. Functionality tested later in stack
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
lib/ops/integrity-store-ops.js | ||
---|---|---|
23 ↗ | (On Diff #37863) | Thanks. This is from thinking we'd need to store the threadHash with the threadHashStatus. I think this is then best kept as a number as I'll change the sqlite table to make threadHash a INTEGER type. |
lib/ops/integrity-store-ops.js | ||
---|---|---|
23 ↗ | (On Diff #37863) | Wondering if this is safe... Looks like for hashing we use string-hash which returns a "number between 0 and 4294967295 (inclusive)" so should be safe for SQLite's INTEGER but I am worried about JSI/WASM parsing, we should use unsigned int to fit maximum number, and this could vary on different platforms. If I remember correctly @atul some time ago suggested always use string in cases like here and then parsing it if we want to store numbers in SQLite. @atul what do you think? |
lib/ops/integrity-store-ops.js | ||
---|---|---|
23 ↗ | (On Diff #37863) | I think we should prefer string vs. number/int in this situation. JS number is internally represented as floating point which isn't 1:1 w/ C++ int/unsigned int etc. There could possibly be issues if like 10.5 (valid JS number) was passed to C++ side and not handled properly, etc. Probably more compelling argument is that using string gives us more flexibility with encoding. We ran into this with eg threadIDs which now are keyserverID|threadID. Similarly could imagine we had hashes encoded with some additional info in the future? Maybe a stretch but we might include information about hash algorithm or something? Like sha512|blah or superQuantumFuturisticHash|blah something? Is there a reason to prefer number/INTEGER for this situation? I think any performance difference should be imperceptible |
lib/ops/integrity-store-ops.js | ||
---|---|---|
23 ↗ | (On Diff #37863) | Then I think it makes sense to convert it to a string. The original reason was just to avoid parsing between number and string and keep the sqlite consistent with our javascript, but what you outlined makes sense. I'll convert from number to string in the next rebase. |
rename ClientDBIntegrityThreadHashesOperation to ClientDBIntegrityStoreOperation for consistency