Perf/optimize get all with json aggregation#805
Merged
roryabraham merged 3 commits intoJul 7, 2026
Merged
Conversation
41 tasks
8adca98 to
cbf2c70
Compare
fabioh8010
approved these changes
Jul 7, 2026
fabioh8010
reviewed
Jul 7, 2026
| // time. Since record_key is the primary key, the rows already come back sorted by key, | ||
| // so ordering by record_key costs almost nothing. | ||
| return provider.store | ||
| .executeAsync<{aggregated: string | null}>('SELECT json_group_array(json_array(record_key, json(valueJSON))) AS aggregated FROM keyvaluepairs ORDER BY record_key;') |
Contributor
There was a problem hiding this comment.
Is the ORDER BY record_key needed for anything?
Contributor
Author
There was a problem hiding this comment.
As mentioned in the comment, it's not strictly necessary - it's only there for readability. Since it's part of the primary key, it's optional. I can remove it and shorten the comment as well
roryabraham
approved these changes
Jul 7, 2026
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Previously getAll() fetched every row from keyvaluepairs and ran JSON.parse once per row (N parses) to hydrate the Onyx cache at startup. It now asks SQLite to aggregate the whole table into a single JSON string.
Related Issues
Expensify/App#89652 (comment)
Linked E/App PR
Expensify/App#95511
Automated Tests
Manual Tests
Author Checklist
### Related Issuessection above### Linked E/App PRsection above, and verified this change against it (E/App CI passed and manual testing completed)TestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Details
Android-getAll-Perf.mp4
Android: mWeb Chrome
iOS: Native
iOS-getAll-Perf.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari