Hotchocolate: EF Core inheritance with IQueryable

[nachtjasmin]

Describe the bug
Assuming we are using the inheritance model of EF Core with the following entities:

public abstract class Product {
    public string Name { get; set; }
}

public class FooProduct : Product {
    public int ProductIndicator { get; set; }
}

public class BarProduct : Product {
    public string AnotherProperty { get; set; }
}

Consider the following query schema:

public class ProductGraph : ObjectType<Product> {}

public class Query : ObjectType
{
    protected override void Configure(IObjectTypeDescriptor descriptor)
    {
        descriptor.Field("products")
            .Type<ListType<ProductGraph>>()
            .Resolver(ctx =>
            {
                var resolver = ctx.Resolver<ProductRepository>() as IEntityListQuery<Product>;
                return resolver?.GetList(); // return type is IQueryable<Product>
            })
            .UsePaging<ProductGraph>()
            .UseSelection();
    }
}

and the rather simple query:

{
  products {
    name
  }
}

Because we've mapped directly to the abstract class, the query is going to fail with an InvalidOperationException and the message Can't compile a NewExpression with a constructor declared on an abstract class.

Expected behaviour
At first glance, this is expected. However, EF Core handles this case by using a discriminator column, so the LINQ query resolver?.GetList()..Select(x => x.Name).ToList() would still work and the abstract class constructor would never been called as the underlying provider would create the Product by using the constructor of FooProduct or BarProduct.

There should be a way of defining this inheritance (union types?) in a comfortable way. Or some documentation tips about creating union type queries, as there documentation lacks information about that in the moment.

What I've tried

1. Removing the abstract keyword from the Product class. Works, but I'd like to avoid that.
2. Defining a union type. Never got it working as expected, might share a sample project later.

[DorianGreen]

I've had a similar experience trying to map a hierarchy as an InterfaceType.
It works nicely, if you do not use selections.

I had to remove the abstract keyword and hack the IdMiddleware to use the discriminator field as the node "type".

If HC implements a discriminator resolver metadata attribute they could use it in the SelectionVisitor and create the proper derived type, instead of the abstract base class.

[michaelstaib]

The resolver metadata are coming with version 11.

[JonasBRasmussen]

@michaelstaib: What exactly is coming in version 11 when you say resolver metadata? Will it be possible to use UseSelection with abstract classes as navigation properties with EF Core?

I just tried to set up a small sample, with the following Query class:

public class Query
{
    [UseSelection]
    public IQueryable<Entry> GetEntries([Service] MyContext myContext) => myContext.Entries;
}

Here Entry is a simple class with a navigation property BaseClasses of type ICollection<BaseClass>, where BaseClass is a simple abstract class having an integer Id property. BaseClass has two subclasses, SubClass1 and SubClass2, configured in an EF Core hierarchy.

I then added an empty BaseClassType inheriting from InterfaceType<BaseClass>, as well as SubClass1Type resp. SubClass2Type, inheriting from ObjectType<SubClass1> resp. ObjectType<SubClass2>, each having the single configuration line descriptor.Implements<BaseClassType>();.

I then try the following query:

query {
  entries { 
    baseClasses {
      __typename # or id
    }
  }
}

The result of which is UseSelection is in a invalid state. Type BaseClass is illegal!.

Is this a scenario that will be solved in version 11 with resolver metadata? Or am I maybe messing up the configuration in some way? I've tried both 10.4.3 and 11.0.0-preview.138.

If helpful, this is my Startup class:

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
        => services
            .AddDbContext<MyContext>(options => options.UseSqlite("Data Source=test.db"))
            .AddGraphQL(
                SchemaBuilder.New()
                    .AddQueryType<Query>()
                    .AddType<BaseClassType>()
                    .AddType<SubClass1Type>()
                    .AddType<SubClass2Type>()
                    .Create());

    public void Configure(IApplicationBuilder app)
    {
        InitializeDatabase(app);

        app
            .UseGraphQL()
            .UsePlayground();
    }

    private static void InitializeDatabase(IApplicationBuilder app)
    {
        using var serviceScope = app.ApplicationServices.GetService<IServiceScopeFactory>().CreateScope();
        var context = serviceScope.ServiceProvider.GetRequiredService<MyContext>();
        if (context.Database.EnsureCreated())
        {
            var entry = new Entry();
            var subClass1 = new SubClass1 { Entry = entry };
            var subClass2 = new SubClass2 { Entry = entry };

            context.Entries.Add(entry);
            context.SubClass1s.Add(subClass1);
            context.SubClass2s.Add(subClass2);

            context.SaveChangesAsync();
        }
    }
}

[bram209]

Any solution on using EF inheritance? I am having a similar issue with UseSelection

[Arsync]

It is not only for EF. Look at this sample:

[KnownType(typeof(Bar))]
public abstract class Foo
{
    public int Id { get; set; }
}

public class Bar : Foo
{
}

And query it with

public class Query
{
    [UseSelection]
    public IQueryable<Foo> GetFoos()
    {
        var foos = new List<Foo>
        {
            new Bar { Id = 1 },
            new Bar { Id = 2 }
        };

        return foos.AsQueryable();
    }

    [UseSelection]
    public IQueryable<Bar> GetBars()
    {
        var bars = new List<Bar>
        {
            new Bar { Id = 1 },
            new Bar { Id = 2 }
        };

        return bars.AsQueryable();
    }
}

GraphQL query:

query {
  foos {
    id
  }
  bars {
    id
  }
}

GraphQL result:
Can't compile a NewExpression with a constructor declared on an abstract class"

And data returned:

"data": {
  "foos": null,
  "bars": [
    {
      "id": 1
    },
    {
      "id": 2
    }
  ]
}

Try to comment [UseSelection] on GetFoos query and you will see normal result without any error.

[Deathrage]

@Arsync well yes because IQueryable extends IEnumarable and using it as such will trigger sync execution (when calling MoveNext on IEnumerator). Using IQuryable without [UseSelection] should be equal as returning IEnumerable (and IEnumerable works alright with inheritance in HC).

However I suppose when using [UseSelection] the execution engine adds to the initial IQueryable creating one big query instead of dozens of small ones (which is when using IQueryable as IEnumerable)

[Arsync]

Never mind of IEnumerable or using IQueryable alone - its just a test for current sample to focus on issue: something wrong exactly within UseSelection logic, not EF-specific. Its about how to reproduce issue with less code / dependencies.

[PascalSenn]

Hi @Arsync, i just replied to @Deathrage in #2484, i just copy over the comment

yes this is currenlty still a limitation. We did not yet had the time to implement polymorphism into projections.
We compile expression trees internally. So a query like

# This query is written in V11 syntax
{
     foos(where: { bar: {eq:400}}, order: [{bar: DESC}, {baz: ASC}]) {
        bar
        baz
    } 
}

is compiled to

foos.Where(x => x.Bar == 400) // UseFiltering
    .OrderByDescending(x => x.Bar) // UseSorting
    .ThenBy(x => x.Baz)  
    .Select(x => new Foo {Bar = x.Bar, Baz =x.Baz}) // UseSelection

As you see we currently create an instance of the requested object. EF traverses this expression tree and then executes the SQL Query and creates the response object. The projection onto the object has a few drawbacks we are aware off.

There are a couple of cases to consider:

• Unions => we need to project completely different types.
• Interfaces => we may need to project onto an abstract type
• Properties without setter => we cannot set these properties in a new expression
• Field Aliases => especially on nested collections this can lead to issues
• Commputed fields => it is currently not supported to project computed fields like
  Field(x => x.Name + " " + x.Surname).Name("fullName")

These are things we want to support, but it is not yet on the board.

[Arsync]

Hi, @PascalSenn. Sad news. Any chance to implement it in V11? Our project heavily relies on polymorphism within domain models.

[PascalSenn]

@Arsync what graphql features do you use? Unions? interfaces?

[Arsync]

Without unions or interfaces for now. We still at beginning of migration from REST to GraphQL. Domain looks like this:

abstract class Department { }
class ProductionDepartment : Department { }
class StockDepartment : Department { }

class Product
{
    public Department Department { get; set; }  // Base class used on Products
}

And querying for fields of base class:

query {
  product {
    id
    name
    department {
      id
      name
    }
  }
}

Of course it can be used without UseSelection, but queries becomes too large for all fields of TPH.

[PascalSenn]

As a workaround for now you could just make this class public. it's definitely not nice and not the final solution, but projections would work in this case. Though when you later us interfaces and unions it will again not work

[Arsync]

Oh, I'm sorry. I've missed 'public' on that classes to show. Currently they public and it still throws Can't compile a NewExpression with a constructor declared on an abstract class, as Department base class is abstract, but queried as field within Product.

[PascalSenn]

@Arsync sorry for the confusion i meant non abstract and a public constructor, so that you can create an instance of it with new Department().
As i said, not nice, but as a workaround maybe good enough

[Arsync]

Just to note: issue is still here with HotChocolate 11.0.0 and its UseProjection attribute.
Non-abstract base class with public constructor is working, but in large domain it may become very painful.