ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.06k stars 723 forks source link

Adding computed fields (Entity Framework) #2365

Open aradalvand opened 3 years ago

aradalvand commented 3 years ago

Hi there, thank you for this great framework! I'm intending to use Hot Chocolate with Entity Framework, and I faced a scenario which I would say is pretty common, but I realized it's apparently not possible to implement with Hot Chocolate.

It's very straight-forward. Let's say you have a Book domain class, and a Rating domain class, and there is a one-to-many relationship between these two. I think it's pretty self-explanatory so far.

Now, I want to add a field on my Book type called rating, which is obviously the average of all the ratings given to a particular book. I thought this would work (this is in the Configure method of the book schema type, as you know)

descriptor.Field(book => book.Ratings.Average(r => r.Rate)).Name("rating");

But it doesn't, and when I query the rating field, I get an Unexpected Execution Error, and it says the Ratings property is null (and I'm aware that this is because it hasn't been fetched from the database).

Any guidance on how I should implement scenarios like this would be highly appreciated. Also, I should mention that since I don't want to face the n+1 problem, I don't want Hot Chocolate to execute book.Ratings.Average(r => r.Rate) separately after it retrieved the book from the database, but preferably it should do this in the single query it creates as well.

michaelstaib commented 3 years ago

Are you using UseSelections?

TheJayMann commented 3 years ago

There are three ways (that I know) to get this to work. First, in your initial query, you need to Include the ratings property so that the query properly does the necessary join operation. The second is for your initial query to invoke the Select method which accesses the Rate property on the Ratings property. Or, perhaps, use a DTO type which has a rating property which assigned in the Select method as defined above, which would also eliminate the need for defining the field.

Finally, there is the UseSelections method, which dynamically constructs a Select method query based on what fields are actually requested. Given that this is not a direct field access, I don't believe it will work by itself. If you are using Entity Framework Core, using UseSelections in conjunction with a DTO having a property assigned to the calculated query does just work, only including the necessary joins if the field was requested, at least in cases that I have tried. I don't know if it works well with Entity Framework as I have not tested this. This method requires the IQueryable provider to have a fairly robust interpretation of the Select method call to cross the DTO boundary, which EFCore appears to do quite well.

aradalvand commented 3 years ago

Are you using UseSelections?

@michaelstaib Yes. I have a Query class that has an IQueryable<Book> GetBooks() method, on which I'm using the UseSelections middleware. Selection on the related Author of a book, for example, works properly.

I think it's reasonable to say that what I've already done should basically work. Given the fact that Hot Chocolate could simply add the expression that I passed as an argument to the Field() method book.Ratings.Average(r => r.Rate) to the Select method query that it generates whenever that rating field is requested. And then EF will take care of generating the right SQL for that expression, of course. Hot Chocolate could do something like this:

... .Select(book => new {
    rating = [The expression that I passed to the Field() method] // If rating is requested, obviously.
}).ToList();

I, of course, don't know the underlying implementation details but I think you get the idea. Please refer to my later comments as well

aradalvand commented 3 years ago

There are three ways (that I know) to get this to work. First, in your initial query, you need to Include the ratings property so that the query properly does the necessary join operation. The second is for your initial query to invoke the Select method which accesses the Rate property on the Ratings property. Or, perhaps, use a DTO type which has a rating property which assigned in the Select method as defined above, which would also eliminate the need for defining the field.

Finally, there is the UseSelections method, which dynamically constructs a Select method query based on what fields are actually requested. Given that this is not a direct field access, I don't believe it will work by itself. If you are using Entity Framework Core, using UseSelections in conjunction with a DTO having a property assigned to the calculated query does just work, only including the necessary joins if the field was requested, at least in cases that I have tried. I don't know if it works well with Entity Framework as I have not tested this. This method requires the IQueryable provider to have a fairly robust interpretation of the Select method call to cross the DTO boundary, which EFCore appears to do quite well.

Hi! The first way you explained won't work for me since I guess it basically first loads all the related ratings of a book from the database and then calculates the average in the application. Instead, I want the right SQL to be generated to get the average of the Rate property on all the related Ratings of a book. Let me know if I misunderstood you.

The second way also won't work for me because I want Hot Chocolate to do the selection dynamically!

But your third way appears to be an interesting solution, but it requires creating extra classes and adding further complexity, also I don't seem to quite understand what you exactly meant when you said "having a property assigned to the calculated query does just work" on the DTO? Do you mean something like this? public double Rating => Ratings.Average(r => r.Rate) Plus, should the DTO inherit from the domain class or what? Thank you for taking the time to write this answer, by the way.

aradalvand commented 3 years ago

Are you using UseSelections?

For example, EntityGraphQL has this exact feature that I'm talking about. You could do something like this:

schema.Type<Book>().AddField("rating", b => b.Ratings.Average(r => r.Rate));

And it would work as expected, without any extra work. It will add that expression to the Select method query it generates, and then all of this will be only one roundtrip to the database (i.e. One SQL query).

I request the rating field and receive it: dsfdf

And here is SQL that gets generated: image

Hot Chocolate too should definitely have this feature.

aradalvand commented 3 years ago

Are you using UseSelections?

Currently, it seems like Hot Chocolate is executing book.Ratings.Average(r => r.Rate) after the rest of the data is fetched from the database, and that's why it's throwing a null reference exception. I don't think that makes sense, book.Ratings.Average(r => r.Rate) should be part of the query that gets generated.

I found this out by selecting the rate fields on the ratings field in the GraphQL query and only then querying the rating field, and as you can see it works: image

But if I don't select the rate field on the ratings field, it's the same story: (because book.Ratings.Average(r => r.Rate) is not part of the generated query) image

TheJayMann commented 3 years ago

I'm assuming your model is something like this.

public class Book {
    public int Id { get; set; }
    public string Name { get; set; }
    public ICollection<Rating> Ratings { get; set; }
}
public class Rating {
    public int Id { get; set; }
    public decimal Rate { get; set; }
}

Assuming this, the DTO would be defined as in the following example.

public class BookDTO {
    // Maybe you do or don't want to copy the Id property over.
    public int Id { get; set; }
    public string Name { get; set; }
    // Maybe you do or don't want to expose the collection in the DTO, and maybe you do or don't want to use a RatingDTO type
    public IEnumerable<Rating> Ratings { get; set; }
    public decimal Rating { get; set; }
}

Then, in your query definition, instead of returning IQueryable<Book>, you would return IQueryable<BookDTO>. Also, you would add a Select before returning the query. It would look something like the following.

public IQueryable<BookDTO> GetBooks(BooksDbContext context) =>
    context.Books
    .Select(b => new BookDTO(){
        Id = b.Id,
        Name = b.Name,
        Ratings = b.Ratings,
        // If you are using a rating DTO type, it would be the following instead
        Ratings = b.Ratings.Select(r => new RatingDTO() { Rate = r.Rate },
        Rating = b.Ratings.Average(r => r.Rate)
    })
;

In my testing, I have found EF Core to be smart enough that, if the query asks for the rating field, then it knows it needs to create SQL to compute the average, and, if the rating field is not requested, then it does not create SQL to compute the average. Also, there are packages out there which exist to make this stuff easier to do, such as AutoMapper, though I have not actually used them for some time, so I cannot verify their compatibility.

aradalvand commented 3 years ago

I'm assuming your model is something like this....

I think your solution is elegant, but for something that should be fairly trivial, I think it's a little bit too complicated. I would rather not create extra DTO classes. I honestly think it's fair to say that what I have already done should be enough to get this to work. And it is in fact enough in some other libraries and frameworks on GrpahQL that I've come across. [See this] I think Hot Chocolate should implement this. And if they don't, I would have to implement your solution. Thank you, again.

TheJayMann commented 3 years ago

Given that my needs require me to implement a DTO anyway (mainly to abstract away many of the complicated database structures underneath, especially many <-> many relationship tables), it tends not to be too much effort for me to implement things this way (i.e. most of the complicated stuff in implementing this will have already been implemented anyway). It is, however, a significant amount of work for a simple case, which is always the case when introducing DTO types, for example, when using Web API. Things like AutoMapper help by doing most, if not all, of this work for you.

I have tested the use of DTOs for situations like this, but only for EF Core. I believe I have tried with EntityFramework without success, and I believe others have tried other IQueryable providers without success. It depends on how robust the IQueryable provider is when it comes to analyzing the query.

Looking briefly at the code, I think it might be possible to enable simple scenarios using the method you described earlier with appropriate changes to the selection middlware. I notice that the Field method does take an expression tree. It depends on how difficult it would be for the selection middleware to see these custom defined fields, and how difficult it would be to attach it to the query generated by the selection middleware.

michaelstaib commented 3 years ago

@AradAral we will look into this ... will take a couple of days. However, any fix will be integrated into version 11. We plan to release it at the end of October. At the moment, Version 11 has no selections support, we started a rewrite of this. @PascalSenn and I will test this against the rewrite.

TheJayMann commented 3 years ago

I just realized an integration like this likely will require a rewrite, as the current selection middleware dynamically generates a Select call where the expression tree lambda passed in uses the same input and output type, and, unless that type has a settable property to hold the result of the expression defined in the custom field, it would have no where to set the expression. A rewrite could possibly generate a lambda which has a dynamically generated anonymous type as its output instead, allowing something like this.

michaelstaib commented 3 years ago

A rewrite could possibly generate a lambda which has a dynamically generated anonymous type as its output instead, allowing something like this.

That's what I was thinking :)

The question is how this can work with our resolver pipeline since resolvers are compiled on startup. I do not want any reflection on runtime.

aradalvand commented 3 years ago

@AradAral we will look into this ... will take a couple of days. However, any fix will be integrated into version 11. We plan to release it at the end of October. At the moment, Version 11 has no selections support, we started a rewrite of this. @PascalSenn and I will test this against the rewrite.

@michaelstaib Ok, thanks.

aradalvand commented 3 years ago

@TheJayMann Thank you so much for the solution you provided, I'm looking into it more and it actually makes more sense than I initially thought. đź‘Ť I just have one question: Given that you create DTOs like that, would you go with the Pure Code-first approach or Code-first approach as Hot Chocolate calls them? For example, would you create a BookType class that inherits from the ObjectType<BookDTO>, or would you just use Hot Chocolate's attributes on the DTO classes' properties? Or is this not relevant at all? I ask this because I personally used the former approach due to my need for creating extra fields that were not part of the domain class, now with DTOs, that need is eliminated. This might be a silly question, so I apologize! Thanks in advance.

TheJayMann commented 3 years ago

I tend to prefer the code first approach, as it provides more flexibility. Mainly because I have also migrated away from using attributes in Entity Framework, instead using the model builder syntax, due to the extra flexibility. However, neither approach really has any benefit over the other in this case, just your own preference of simple syntax or flexibility.

aradalvand commented 3 years ago

@TheJayMann Got it. Thank you again for taking the time to help. I really appreciate it.

aradalvand commented 3 years ago

@TheJayMann There is a serious problem that I found with using DTOs here which is that they wouldn't work properly with selections. If I have this GetBooks method:

public IQueryable<BookDto> GetBooks([Service]AppDbContext dbContext)
{
    return dbContext.Books.Select(book => new BookDto
    {
        Name = book.Name,
        ...
        Author = new AuthorDto
        {
            Id = book.Author.Id,
            Name = book.Author.Name,
        }
    });
}

And I request something like this:

{
  books {
    name
    author {
      name
    }
  }
}

Then this will be the generated SQL:

SELECT [b].[Name], [a].[Id], [a].[Name]
      FROM [Books] AS [b]
      INNER JOIN [Authors] AS [a] ON [b].[AuthorId] = [a].[Id]

And as you see it retrieves the ID of the author even though I didn't query it. If any field on the author is queried, then it basically retrieves all the fields of the author. It gets worse with collections.

Do you think there is any way to solve this? Thanks in advance.

PascalSenn commented 3 years ago

@AradAral have you tried it with more than one field on the author?

aradalvand commented 3 years ago

@PascalSenn Well, yes. No matter how many fields I select on the author (1 or 100), it would retrieve the whole thing

aradalvand commented 3 years ago

@PascalSenn this is probably related to Hot Chocolate. Because when I tried the following:

return dbContext.Books.Select(book => new BookDto
            {
                Name = book.Name,
                Author = new AuthorDto
                {
                    Id = book.Author.Id,
                    Name = book.Author.Name,
                }
            }).Select(bDto => bDto.Author.Name).FirstOrDefault();

Then this was EF core's generated SQL:

SELECT TOP(1) [a].[Name]
      FROM [Books] AS [b]
      INNER JOIN [Authors] AS [a] ON [b].[AuthorId] = [a].[Id]

As you can see, it's properly only fetching the name of the author and nothing else. But it doesn't work properly with the UseSelections middleware of Hot Chocolate.

Should I post another issue for this?

PascalSenn commented 3 years ago

@AradAral hmm.. interesting.

how does your resolver look like? what is the order of the middleware's?

Btw, we do project it directly to the field.
We project it like this:

{
  books {
    name
    author {
      name
    }
  }
}
.Select(x=> new BookDto
            {
                Name = x.Name,
                Author = new AuthorDto
                { 
                    Name = x.Author.Name,
                }
            })
aradalvand commented 3 years ago

@PascalSenn I just have one middleware on the GetBooks method/field and that is UseSelection. And I also posted my implementation of the GetBooks method right here. And that's basically it! The project is very very simple, nothing fancy or complicated, really.

I also executed the code that you said Hot Chocolate generates, and I get a different SQL: This code:

var queryableDto = dbContext.Books.Select(book => new BookDto
{
    Name = book.Name,
    Author = new AuthorDto
    {
        Id = book.Author.Id,
        Name = book.Author.Name,
        Age = book.Author.Age
    }
});
// The code that you said Hot Chocolate would generate:
queryableDto.Select(bDto => new
{
    Name = bDto.Name,
    Author = new
    {
        Name = bDto.Author.Name,
    }
}).ToList();

Generates the following SQL:

SELECT [b].[Name], [a].[Name]
      FROM [Books] AS [b]
      INNER JOIN [Authors] AS [a] ON [b].[AuthorId] = [a].[Id]

Which doesn't retrieve all Author fields, only the requested ones (in this case just Name). Given the same GraphQL query, Hot Chocolate generates the following SQL:

SELECT [b].[Name], [a].[Id], [a].[Name], [a].[Age]
      FROM [Books] AS [b]
      INNER JOIN [Authors] AS [a] ON [b].[AuthorId] = [a].[Id]

Which as you see retrieves all the columns of the author, including Age and Id which weren't queried. This might be a bug or something.

We project it like this:...

So I guess you probably don't project it like that :)

aradalvand commented 3 years ago

@PascalSenn Should I file a separate issue for this?

AaronFlynn1989 commented 3 years ago

@PascalSenn @michaelstaib Hey. I'm sorry i wanted to ask whether this is actually going to be implemented in the next version or not ? Very briefly : we have a finance-related web app, we were thinking about swtiching to GraphQL from traditional REST. The app revolves a lot around running custom/complicated queries, and we're using EF core . we stumbled into hot chocolate and i also read some of your docs + the blog post on your website about how hot chocolate can work with entity framework , and we were doing some testing to see whether it fits our needs . we did the same thing as the OP and totally expected it to work, i thought it would add our expression to the final projection expression that's going to be given to EF to generate the SQL and... which you guys have apparently already discussed. but then we realized it doesn't, i thought maybe we were doing it wrong or making a mistake, i googled the problem and found this issue, but i was disappointed to find out this is not currently possible in hot chocolate, and quite honesly i was also surprised since i think you'd agree it's a fairly common scenario, for any app that's a little more complicated than a simple to-do app!

i don't wanna make this too long, i just had one question, have you looked into this yet, as you said, and are you actually planning to implement and release it in the next version ? or it's not certain yet ? this feature is deal breaker for us (and i think most other apps who want to use hot chocolate with entity framework) and we would have to heavily depend on it.

michaelstaib commented 3 years ago

Hi @iamaaronflynn,

while we have computed fields on our roadmap we have no exact date for it. It is not planned for version 11 (October Release). Also, let me remind you that this project is an open-source project that is done by people in their free time. If this feature is business-critical to you, you can always choose to contribute a missing feature.

For version 11 we have focused on a different feature set since only 10% of our users do really use projections and in general, asked for different features like the data integration layer. We do have an open slack channel where our community congregates and votes from time to time on features they want in Hot Chocolate. We invite you to join an take part in this community and also to contribute to it.

What I ask from anyone who joins our community is to be respectful and kind since the people who worked on these features really put a lot of work into them (in their free time) and sentences like:

but i was disappointed to find out this is not currently possible in hot chocolate, and quite honesly i was also surprised since i think you'd agree it's a fairly common scenario, for any app that's a little more complicated than a simple to-do app!

If you really need a new feature in a timely manner and you do not have the time to contribute to an open-source project there is always the possibility to pay for features or support.

Last but not least there might also be workarounds to get this done with the current version, for this really ask in the slack channel since EF is not my area of expertise.

Here is the link to join: https://join.slack.com/t/hotchocolategraphql/shared_invite/enQtNTA4NjA0ODYwOTQ0LTViMzA2MTM4OWYwYjIxYzViYmM0YmZhYjdiNzBjOTg2ZmU1YmMwNDZiYjUyZWZlMzNiMTk1OWUxNWZhMzQwY2Q

AaronFlynn1989 commented 3 years ago

@michaelstaib sorry if what I said sounded disrespectful, that was honestly not my intention and I didn't mean to be rude or anything like that, so I'm sorry if I sounded so. I do think this is valuable work, and I do certainly appreciate everyone involved. And I definitely didn't expect you to do anything, since I'm not a paying customer I don't have that right. And I agree with pretty much everything you said.

Anyway, I wait and hope to see this feature in hot chocolate soon, this is a great framework and has a lot of potential, and the above-mentioned feature would definitely make it way more useful and rich. Thank you all.

aradalvand commented 2 years ago

[Dummy comment to prevent the stale bot from closing the issue]

aradalvand commented 1 year ago

.

aradalvand commented 1 year ago

(activity)

kolpav commented 1 year ago

Hello @michaelstaib I haven't been following HC for a while so I am curious if its now possible/easier to make derived/computed fields work with sorting/filtering somehow?

I know its very hard problem to solve at the same level of quality as rest of the HC features so I am ok with any solution to this problem even if its not exactly ideal, thank you.