Projections: Support selecting only __typename on union types
Summary of the changes (Less than 80 chars)
- Support selecting only __typename (without any other 'real' members) on union type
Closes #8252 (in this specific format)
Background:
If no 'real' member of an entity was requested as part of the selection set, like in
{
foo {
items { # This is a union of ConcreteType1|ConcreteType2
__typename
... on ConcreteType2 {
foo
}
}
}
}
the projected query currenty looks like this:
repository.Select(_s1 => (_s1 is ConcreteType1) ? new ConcreteType1 {}
: (_s1 is ConcreteType2) ? (ConcreteType2)new ConcreteType2 { Foo = ((ConcreteType2)_s1).Foo }
: default(BaseType));
However, new ConcreteType1 {} cannot be translated to SQL, which leads to the following exception:
The client projection contains a reference to a constant expression of 'ConcreteType1'. This could potentially cause a memory leak; consider assigning this constant to a local variable and using the variable in the query instead.
I fixed this by transforming the query into:
repository.Select(_s1 => (_s1 is ConcreteType1) ? (BaseType)_s1
: (_s1 is ConcreteType2) ? (ConcreteType2)new ConcreteType2 { Foo = ((ConcreteType2)_s1).Foo }
: default(BaseType));
This approach selects the entire record, so it indeed overfetches. Anyways, it seems to be the simplest solution that works in all cases.
I considered selecting just one property instead, but that seems to be hacky and tricky since we cant know for sure whether a property is actually part of the table or just a calculated helper property. Because we’re only selecting the values of the base type and also do not eager load navigation properties, I believe the performance penalty from this overfetching is negligible compared to correctness. Also this seems to be a rare edge case.
However, if another approach is preferred - like the one with just selecting one property-, please let me know and I can reevaluate this.
@N-Olbert we at the moment do not touch anything regarding HotChocolate.Data or GreenDonut.Data as we plan to bring them together. We will pull out the projections engine and rearchitecture it. I can involve you into that process, but it will be somewhere in November as we are focused on Fusion at this point.
@michaelstaib sure, ping me then.
Do you think it makes sense to add the (failing or skipped) tests now, so that this case won't be forgotten then?