-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix(@aws-amplify/datastore): improve IDB query performance #7746
Conversation
Codecov Report
@@ Coverage Diff @@
## main #7746 +/- ##
==========================================
+ Coverage 74.07% 74.13% +0.05%
==========================================
Files 214 215 +1
Lines 13410 13450 +40
Branches 2628 2641 +13
==========================================
+ Hits 9934 9971 +37
- Misses 3277 3280 +3
Partials 199 199
Continue to review full report at Codecov.
|
31df8ce
to
e36ee36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me. The refactor is much easier to follow. Although what do you think about doing the same thing for AsyncStorageAdapter
for consistency's sake?
e36ee36
to
1e4f0b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great!
I left 2 comments but they are non-blocking, let me know your thoughts
1e4f0b2
to
e44cdff
Compare
@amhinson Agreed! I've refactored |
87bca91
to
5fb1d56
Compare
@amhinson @manueliglesias - thanks for the thorough reviews and good suggestions, guys. I've updated the PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
3b629d4
to
e5df7b2
Compare
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
This PR fixes a bug where calling
DataStore.query(Post)
without a pagination object would use the cursor-based fetch from IndexedDB, as opposed togetAll()
, which is optimized specifically for getting all the records. This was having a negative effect on the performance ofDataStore.query
when retrieving all the items for a given model.The pagination object was getting initialized to
{limit: undefined, page: undefined, sort: undefined}
in theDataStore.processPagination
private method, thereforepagination
always had a truthy value in the conditional checks that determine which retrieval method to utilize. This gets fixed here.Also:
IndexedDBAdapater.query
to make it easier to testIndexedDBAdapter.query
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.