graphql-go
graphql-go copied to clipboard
graph-gophers
mixed concrete types with pointer receivers are unsupported.
I have a resolver returning a list of resolvers like this:
type resolver struct {}
func (r *resolver) Foos() ([]fooResolver, error) {
// ... implementation
}
type fooResolver struct{}
func (r *fooResolver) ID() graphql.ID
When I try to query with the schema above, I get an error like this:
parsing scheme: resolver.projectResolver does not resolve "Foo": missing method for field "id" (hint: the method exists on the pointer type)
returned by (*resolver.resolver).Foos
The code mapping these types shouldn't care whether or not there is a pointer when validating if a type implements an interface. Since my resolvers are shallow types, leaving them concrete in the struct will reduce pointer lookups. However, I'd like to use a pointer receiver on these methods to avoid the copy for each method.
To make this code work with a pointer receiver, I'd have to return a slice of pointers, which doesn't really make a whole lot of sense, unless there is expense to copying the type or they are shared in some way.
There are two points of inefficiency:
- When creating a slice of pointers, we now have an allocation for each member of the slice, as well as the slice allocation. There should only be a single allocation for a shallow resolver type when creating a slice.
- Additional cache pressure on the unnecessary layer of pointer lookups.
While these seem like small issues, they can contribute to performance problems by generating additional allocations and slow performance on cache line misses.
It looks like the issue is https://github.com/graph-gophers/graphql-go/blob/e582242c92cc545085df10b3598575a912e118a2/internal/exec/resolvable/resolvable.go#L233. It seems to make the assumption that a type can only have a method if it is a pointer type, when it should not care.
As far as I understand it, when a type is nullable in the GraphQL schema, then it needs to be a pointer in Golang. Am I missing something?
As far as I understand it, when a type is nullable in the GraphQL schema, then it needs to be a pointer in Go. Am I missing something?
Nullability doesn't really come into play here. If you want to represent a null slice, you just set it to null. While you can have a schema with nullable elements, it doesn't make a lot of sense in practice. In the case of this schema, the elements are not declared as nullable, so allowing a concrete resolver should be allowed.
Put into an example, we should be able have the following work, without modifying the type of the receiver:
type fooResolver struct{}
func (f *fooResolver() MyNullableBars() []*barResolver
func (f *fooResolver() MyConcreteBars() []barResolver
type barResolver struct{}
func (r *barResolver) MyField() string { return "bar" }
With the above, we have a barResolver that can be used in a nullable or non-nullable context. With implementation the way it currently is, I'd have to implement one with a pointer receiver and one with a concrete receiver, which shouldn't matter.
This lets us avoid copying for method calls and control nullability.
With implementation the way it currently is, I'd have to implement one with a pointer receiver and one with a concrete receiver, which shouldn't matter.
Well, how often do you need nullable and non-nullable version of the same resolver in the same schema? Again, the fact that it hasn't happened to me doesn't mean that it is not a real issue.
This lets us avoid copying for method calls and control nullability.
How does this let us control nullability?
I am still not convinced that this is a real issue.
I ran into the same situation. I tried to implement a schema with non-nullable resolvers, but had to re-implement my resolver's pointer receivers as value receivers, which comes with performance hits.
@jbacon out of curiosity what was the performance hit? Also, why did you have to change the resolver's pointer receivers? You could as well dereference the pointer in the resolver.
just ran into a similar problem - an example below schema:
type Query {
helloWorldOne: HelloWorld
}
type HelloWorld {
hello: String!
}
resolver:
func (r *RootResolver) HelloWorldOne() *HelloWorldResolver {
return &HelloWorldResolver{}
}
type HelloWorldResolver struct {
// empty in this example
}
func (hw *HelloWorldResolver) Hello() string {
return "Hello World"
}
Everything above works fine. However, if we I want to add a second query which returns non-nullable type
type Query {
#helloWorldOne: HelloWorld
helloWorldTwo: HelloWorld!
}
func (r *RootResolver) HelloWorldTwo() HelloWorldResolver {
return HelloWorldResolver{}
}
I get the following error:
panic: HelloWorldResolver does not resolve "HelloWorld": missing method for field "hello" (hint: the method exists on the pointer type)
used by (*resolvers.RootResolver).HelloWorldTwo
In my project, I ran into this problem with some custom scalar involved and got a super vague panic message - took me a while to find out this root cause
Since GoLang makes the pointer receiver method accessible to both pointer & concrete types, I think it should be the same behavior for resolving both nullable & non-nullable fields in graphql.
To rephrase: I think ideally the method on pointer type can also be resolved by concrete type.
@pavelnikolov do you think it makes sense? If so, I can start looking into making the update
