graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Projections: Support selecting only __typename on union types

Open N-Olbert opened this issue 3 months ago • 2 comments

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 avatar Sep 16 '25 17:09 N-Olbert

@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 avatar Sep 17 '25 08:09 michaelstaib

@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?

N-Olbert avatar Sep 17 '25 10:09 N-Olbert