-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support _deleted selector in RxDB queries #18
Conversation
7cccbf8
to
d89783a
Compare
@@ -816,7 +828,12 @@ async function __ensureEqual<RxDocType>(rxQuery: RxQueryBase<RxDocType, any>): P | |||
} | |||
} | |||
|
|||
if (rxQuery.op === 'count') { | |||
if (rxQuery.includesDeleted) { |
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.
I can't quite remember, but do we need to specifically handle count
operations for any selector that contains a true-like value for _deleted
and supports counting via an index (fast count). Or is _deleted
always a slow count?
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.
Pseudo code
} else if (rxQuery.op === 'count' && rxQuery.includesDeleted) {
rxQuery._execOverDatabase().then((newResultData) => {
rxQuery._setResultData(newResultData ? newResultData.length : 0);
return true;
});
}
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.
This is a good point that I have not tested yet. Could I verify this by using the field that Tristan mentioned here 'rxdbOnly.indexFields.triage_status_exists'
and then check the correct count?
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.
This is a good point that I have not tested yet. Could I verify this by using the field that Tristan mentioned here
'rxdbOnly.indexFields.triage_status_exists'
and then check the correct count?
Yeah… I think this could work, but it is also possible that the query planner “sees” the _deleted
field in the selector, and just skips finding the best index, because it is an “unknown field” for the query planner. If you use rxdbOnly.indexFields.triage_status_exists
and it actually picks up this index, you can potentially log the usage. https://github.com/readwiseio/rekindled/blob/ac42918be6d214fab78344a115405c9bc39ab3b7/reading-clients/shared/database/internals/storages/storage-sqlite/sqlite-storage-instance.ts#L360 for mobile, you could add a console.log
and check which index is picked. It is possible though that no index, which results in RxDB to fall back to the default (this should be id
).
LGTM. One logic question about slow vs. fast count branch in RxDB, but I assume you already tested this and it is fine. |
Allows deleted documents to be queried when including the
_deleted
selector.