-
Notifications
You must be signed in to change notification settings - Fork 472
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
feat(adapter/kv): support async iterating on scan results #5208
Conversation
d38c7ec
to
b014ca6
Compare
core/src/services/sqlite/backend.rs
Outdated
#[self_referencing] | ||
pub struct SqlStream { | ||
pool: SqlitePool, | ||
query: String, | ||
|
||
#[borrows(pool, query)] | ||
#[covariant] | ||
stream: BoxStream<'this, Result<String>>, | ||
} |
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.
The lifetime handling here is a bit tricky.
sqlx::query_scalar()
and its .fetch()
heavily depend on lifetime parameter restrictions:
pub fn fetch<'e, 'c: 'e, E>(self, executor: E) -> BoxStream<'e, Result<O, Error>>
where
'q: 'e,
E: 'e + Executor<'c, Database = DB>,
DB: 'e,
A: 'e,
O: 'e,
So here to extend the lifetime (since we don't use GAT in our API here e.g. type ScanIter<'a>
, in order to be compatible to trait List
which is also not a GAT in Access
.), we need to carry both the Pool
and also the query &str
along our Stream
into return value and put it into a self-referencing structure and then we don't need to care the lifetime checking.
2b23278
to
241a1e7
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.
Thank you, @PragmaTwice, for this excellent pull request!
There are some critical bug fixes that need to be released. I will merge this after we create a new release. |
Which issue does this PR close?
Closes #3639.
Rationale for this change
Instead of performing a full scanning and returning a
Vec<String>
, now we introduce an associated type intokv::Adapter
:So that the implementation can do paged scanning and returns a
ScanIter
, and users can iterate over it to get keys in batches from the server.What changes are included in this PR?
ScanIter
intokv::Adapter
and refactorKvLister
andAdaptor
'sAccessor
impl;kv::Adapter
, especially for which has list cap;fetch_all
, we can usefetch
to fetch data in batches instead of fetching all data.Are there any user-facing changes?
No changes for end users.