-
Notifications
You must be signed in to change notification settings - Fork 506
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
[REQUEST] Add batch grouping #359
Labels
Comments
probably it's a better idea to select all the statuses in one query, reducing multiple calls to the db. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem are you trying to solve?
Handling different parameters in field resolvers with dataloader is tricky.
Let’s have this GraphQL schema:
Then we have query:
Here orders resolver with dataloader:
Here ordersLoader with batchLoadFn:
In this batch loader function we need to get orders from a database. If we have all orders stored in one table we can have single query, like:
Then loop through params and return the result. No real issues, so far.
Now let’s think orders in different statuses can’t be queried with a single query. For example, “New” orders are in “Online” db and “Competed” are moved in some “Archive” db. So, we need to have 2 queries. Then we need to build results properly based on input params. It is doable but even with this simple example not so easy. In real life we may have multiple parameters and we need to handle all permutations of them.
Describe the solution you'd like
I suggest adding a new option to DataLoader constructor. Very similar to existing cacheKeyFn – batchGroupFn?: (key: K) => C;. Based on result of this function batchLoadFn function needs to be called as many times as many different results gives batchGroupFn. I.e. I want to group batches by input keys.
For my example it would be:
So for each status it will be separate call of batchLoadFn. It will simplify this function a lot.
Please let me know if this solution is feasible. Or maybe there is alternative/better solution?
The text was updated successfully, but these errors were encountered: