Support list fields in selections with type conditions (inline fragments)

[MarianPalkus]

Hi chad, i noticed that list fields in selections with type conditions/inline fragments are not working yet. Here is an example:

• Data models:

  
  public class Character
  {
      public int Id { get; set; }
      public string Name { get; set; }
  }
  
  public class Human : Character
  {
      public double Height { get; set; }
      public ICollection<Vehicle> Vehicles { get; set; }
  }
  
  public class Stormtrooper : Human
  {
      public string Specialization { get; set; }
  }
  
  public class Droid : Character
  {
      public string PrimaryFunction { get; set; }
  }
  
  public class Vehicle
  {
      public int Id { get; set; }
      public string Name { get; set; }
      public int OwnerId { get; set; }
  }

• Considering the following GraphQL query:

  { 
  heros {
    name
    __typename
    ... on Human { 
       height
       vehicles { name }
    }
    ... on Stormtrooper { 
       specialization 
    }
    ... on Droid { 
       primaryFunction 
    }
  }
  }

  ...where we select the list field vehicles on heros of type Human. The result may look like this:

  { 
  heros: [
  { name: 'Han Solo', __typename: 'Human',  height: 5.6430448, vehicles: [ {name: 'Millennium falcon'}] },
  { name: 'FN-2187', __typename: 'Stormtrooper',  height: 4.9, vehicles: [ {name: 'Speeder bike'}], specialization: 'Imperial Snowtrooper'},
  { name: 'R2-D2', __typename: 'Droid', primaryFunction: 'Astromech' } 
  ] 
  }

When executing the query the following exception is thrown:

in \graphql-net\Tests\InMemoryExecutionTests.cs : line 36

....
in graphql-net\Tests\InMemoryExecutionTests.cs:line 36
Result Message: System.ArgumentException : Argument types do not match

Looking at the GraphQl.Net/Executor.cs line 296ff:

            if (needsTypeCheck)
            {
                var typeCheck = Expression.TypeIs(baseBindingExpr, field.DefiningType.CLRType);
                var nullResult = Expression.Constant(null, toMember.PropertyType);

                // The expression type has to be nullable
                if (replacedBase.Type.IsValueType)
                {
                    replacedBase = Expression.Convert(replacedBase, typeof(Nullable<>).MakeGenericType(replacedBase.Type));
                }

                replacedBase = Expression.Condition(typeCheck, replacedBase, nullResult);
               }

The toMember is a list type, it has the type of the elements of the list (e.g. String if the list is of type ICollection<String>).

This can be fixed by using replacedBase.Type instead:

            if (needsTypeCheck)
            {
                // The expression type has to be nullable
                if (replacedBase.Type.IsValueType)
                {
                    replacedBase = Expression.Convert(replacedBase, typeof(Nullable<>).MakeGenericType(replacedBase.Type));
                }

                var typeCheck = Expression.TypeIs(baseBindingExpr, field.DefiningType.CLRType);
                var nullResult = Expression.Constant(null, replacedBase.Type);
                replacedBase = Expression.Condition(typeCheck, replacedBase, nullResult);
               }

This works fine for in-memory tests, however, for EF this results in the following error:

graphql-net\Tests.EF\EntityFrameworkExecutionTests.cs : line 235

...
at System.Data.Entity.Core.EntityClient.Internal.EntityCommandDefinition..ctor(DbProviderFactory storeProviderFactory, DbCommandTree commandTree, DbInterceptionContext interceptionContext, IDbDependencyResolver resolver, BridgeDataReaderFactory bridgeDataReaderFactory, ColumnMapFactory columnMapFactory)
Result Message: 
System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
  ----> System.Data.Entity.Core.EntityCommandCompilationException : An error occurred while preparing the command definition. See the inner exception for details.
  ----> System.NotSupportedException : The nested query is not supported. Operation1='Case' Operation2='Collect'

This seems to be a more complex issue. I think the sql query has to be split up into more sql queries.. but i do not have an idea to do this in a nice way.

Here is a branch with related unit tests and a first approach to fix this issue: https://github.com/MarianPalkus/graphql-net/tree/b-inline-fragments-with-list-field

Do you have any idea how to solve this?

[chkimes]

Hi Marian,

Thanks for all the detail! This is a pretty tricky issue, but ultimately I think the blame lies with Entity Framework. They are trying to translate this into SQL and having issues:

Vehicles = p is Human ? (p as Human).Vehicles.Select(...) : null

It comes down to EF having a strange model for handling inheritance, which is yet again different from how it works in memory. What they expect to see is to just use a cast with no type check:

Vehicles = (p as Human).Vehicles.Select(...)

Since this is another special case handling for EF, I think we should add this as another option to ExpressionOptions. I've done that on top of your code changes and tests here:

https://github.com/ckimes89/graphql-net/tree/b-inline-fragment-list-field-fix

Let me know what you think. If it works out with your use case, you can send it in as a PR and I'd be happy to accept it.

[MarianPalkus]

Thanks for the fast response and the solution, looks great :+1: I will try it out and create a Pullover request.

[MarianPalkus]

The solution works as expected.

However, i have to set the typeCheckInheritance option to false, if i understand you correctly, this is generally necessary for EF, isn't it?

In GraphQlSchema line 107:

WithExpressionOptions(t => t.FullName.StartsWith("System.Data.Entity.Infrastructure.DbQuery"), castAssignment: false, nullCheckLists: false, typeCheckInheritance: true);

The typeCheckInheritance option is set to true when using EF.

Should it be set to falseinstead?

WithExpressionOptions(t => t.FullName.StartsWith("System.Data.Entity.Infrastructure.DbQuery"), castAssignment: false, nullCheckLists: false, typeCheckInheritance: false);

The related unit test EntityFrameworkExecutionTests.InlineFragementWithListField succeeds, because the default expression options are used, since the predicate of the EF-expresion options is false:

t.FullName.StartsWith("System.Data.Entity.Infrastructure.DbQuery")

In case of this unit test, t starts with System.Data.Entity.DbSet.

So, another question regarding the expression types: 'Simple' fields like schema.AddListField("heros", db => db.Heros) seem to be of type System.Data.Entity.DbSet and 'complex' fields like schema.AddListField("heros", db => db.Heros.AsQueryable().Where(h => true)) seem to be of type System.Data.Entity.Infrastructure.DbQuery.

Is this assumption correct? Should we extend the predicate for EF expression options to:

WithExpressionOptions(t => t.FullName.StartsWith("System.Data.Entity.Infrastructure.DbQuery") || t.FullName.StartsWith("System.Data.Entity.DbSet"), castAssignment: false, nullCheckLists: false, typeCheckInheritance: false);

[chkimes]

Good catch, typeCheckInheritance should definitely be false for EF.

That's interesting about the query types, I wasn't aware that there were multiple. I think just simplifying the type check to starting with System.Data.Entity will be sufficient, since I don't think any other ORMs plan to use that namespace and it will catch any other unknown types of an EF query.