-
-
Notifications
You must be signed in to change notification settings - Fork 523
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
[PIP] Simple data loader #1238
[PIP] Simple data loader #1238
Conversation
* modify signature to accept statement outside * add simple test
Its not ready yet. I need to do some testing |
The implementation looks great! Made a few improvements:
May be we still want more test cases? I am tempted to change |
Yes we can come back to this after we got a Also may be awaiting @billy1624 's review |
Can we also test this behaviour? I might be wrong but I suspect the current implementation would panic |
Created issue for HashableValue |
Regarding #1238 (comment) It didn't panic, but it was returning None. I fixed it by copying the values before returning. |
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.
Hey @karatakis, sorry for coming in late. nice stuffs!! Thanks for the hard work!!
I think we should also implement the loader for M: ModelTrait
. Providing a "complete" API, Vec<M>
implemented LoaderTrait
, M
itself should also implement it.
Load one
let bakers = baker::Entity::find()
.one(&ctx.db) // `one` here, instead of all
.await?;
let bakeries: Option<_> = bakers // `Option<_>` here, instead of `Vec<Option<_>>`
.load_one(bakery::Entity::find(), &ctx.db)
.await?;
Load many
let bakeries = bakery::Entity::find()
.one(&ctx.db) // `one` here, instead of all
.await?;
let bakers: Vec<_> = bakeries // `Vec<_>` here, instead of `Vec<Vec<_>>`
.load_many(baker::Entity::find().filter(baker::Column::Name.like("Baker%")), &ctx.db)
.await?;
That means we need two extra associate types to type def the return type of both methods.
pub trait LoaderTrait {
// Existing associated type
type Model: ModelTrait;
// New associated type to type def the return types
type LoadOneResult;
type LoadManyResult;
Good point, although I think we don't have to mess the implementation, I suppose we can simply provide an implementation that just wraps a Vec of 1 item. I want to merge this for now for 0.11 |
PR Info
Allows to query related entities from vector
Load one
Load many
New Features