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
Potential optimization issues with BatchMapping #869
Comments
…try alongside BiFunction registration methods. resolves spring-projects#870 & spring-projects#869
It is up to the parent type For example replace this: @Controller
public class BatchListController {
@QueryMapping
public Collection<Course> courses() {
// ...
}
@BatchMapping
public List<Person> students(List<Course> courses) {
// ...
}
} with this: @Controller
public class BatchListController {
@QueryMapping
public Collection<Long> courses() {
// return course id's
}
@BatchMapping(typeName = "Course")
public List<Person> students(List<Long> courseIds) {
// ...
}
} |
While that may be true of the implementation provided, that is not necessarily how the data loader library works for data leaders with mapped batch loaders. That is part of the point of using a mapped batch loader, no? It makes it so that I don't have to worry about whether I have duplicates in the provided keys, because it is a Set. With the course id example, does it automatically pass the ids of a type as the parameters of a batch mapping method if you have a signature and annotation like you have shown? I guess that I'm struggling to see how it knows what to pass as the argument unless it's just looking for an "id" field or getter. Also, if the courses field is a list of IDs, then how would it resolve the entire type when selected? Does it resolve the entire type and not just the ids? Another issue with how things are implemented is that this approach also seems to encourage inefficient use of data/batch loaders. Here is one example: Let's say that I have two different types A and B. While both types aren't related to each other, they do share a similar field: createdBy. This field is of type User. If a query is submitted where both types are selected at the same nested level in the query and the createdBy field is selected for both types, multiple queries would end up being executed to fetch the User types instead of one. This is because two different batch/data loaders would be created, one for type A and one for type B, even though they are fetching user data with a user id (assuming but A and B types use a user id to fetch the value of createdBy). The data loader library is meant to be used such that a single data loader is used to fetch this data when resolving these two different yet similar fields. On top of this, it also reduces the amount of potential cache hits in a data loader during a single query resolution since multiple loaders would be used instead of a single loader. Here is an example of the query described above: # "createdBy" field of "a", "b", and "c" fields
# and the "modifiedBy" field of "c"
# should resolve using a single fetch via a
# single data loader, but they won't, they will
# resolve using 4 separate data loaders.
query {
a { # type is A
createdBy { # will result in separate fetch
...user
}
}
b { # type is B
createdBy { # will result in separate fetch
...user
}
}
c { # type is C
createdBy { # will result in separate fetch
...user
}
modifiedBy { # will result in separate fetch
...user
}
}
}
fragment user on User {
id
name
} |
BatchMapping injects a set or list of the type which owns the field being resolved. This presents a problem: if a set of objects is provided and there isn't an appropriate equals override in the type, then you could get several of essentially the same object though with different memory references, which would cause the need to either find the truly unique values that you care about or would result in significantly more data being used in a query that would potentially take place.
It's been my experience that one should stick to more "scalar" types for keys like strings and numbers. An example would be getting all of the children of a parent by parent IDs rather than by the parents themselves.
The text was updated successfully, but these errors were encountered: