Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure IQueryable implements IAsyncEnumerable before using ToListAsync #4018

Closed
WolfspiritM opened this issue Jul 31, 2021 · 3 comments
Closed
Labels
Area: EntityFramework Issue is related to the EntityFramework integration 🌶️ hot chocolate
Milestone

Comments

@WolfspiritM
Copy link

WolfspiritM commented Jul 31, 2021

Describe the bug
When using EFCore with UseDbContext and returning an IQueryable that is not coming from EFCore (e.g. does not implement IAsyncEnumerable), then the query will fail with the error The source 'IQueryable' doesn't implement 'IAsyncEnumerable<Ocora.Services.Telematics.Core.Entities.Masterdata.Dtos.AssetDto>'. Only sources that implement 'IAsyncEnumerable' can be used for Entity Framework asynchronous operations.

I know that in general always the database IQueryable should be returned for performance reasons and possible filtering but in some cases this might not be possible and for debugging it can also be helpful to return a list instead of the IQueryable.

The error comes from here: https://github.com/ChilliCream/hotchocolate/blob/c1eb36cf3542572372847c5e38f4bc349bc22bfa/src/HotChocolate/Data/src/EntityFramework/ToListMiddleware.cs#L23-L25

I think this needs a check for IAsyncEnumerable similar to https://github.com/ChilliCream/hotchocolate/blob/c1eb36cf3542572372847c5e38f4bc349bc22bfa/src/HotChocolate/Data/src/Data/QueryableExecutable.cs#L44

Maybe this is even a bug as it's not using that extension method but the EFCore internal extension method cause of a missing/wrong using statement?!

To Reproduce
Steps to reproduce the behavior:

  1. Add UseDbContext
  2. return an enumerable (e.g. a list) with ".AsQueryable()".

Expected behavior
No exception as ToList is used instead of ToListAsync if IQueryable does not implement IAsyncEnumerable

Desktop (please complete the following information):

  • OS: [e.g. iOS] Windows 10
  • Version [e.g. 22] 11
@tobias-tengler tobias-tengler added Area: EntityFramework Issue is related to the EntityFramework integration 🌶️ hot chocolate labels Jul 31, 2021
@michaelstaib michaelstaib added this to the Backlog milestone Nov 22, 2021
@PascalSenn PascalSenn added the 🙋 good first issue Good for newcomers label Mar 14, 2022
@gkfischer
Copy link
Contributor

@michaelstaib I'm wondering if a type check like if (Source is IAsyncEnumerable<T> ae) is really a good idea in terms of performance because it would add that type check for every request. I think returning a list.AsQueryable() is the rare exception, isn't it? If so I would rather try to catch the exception and respond to it with 'queryable.ToList(). That's pretty slow in comparison to the if` but only kicks in when there is no Quyerable coming from EF.

@michaelstaib
Copy link
Member

The ToListMiddleware is in the EFCore package so we expect it to not work on anything else than EFCore. I think we can close this one. Since its actually not a bug.

@michaelstaib michaelstaib added ❓ question This issue is a question about feature of Hot Chocolate. and removed 🐛 bug 🙋 good first issue Good for newcomers labels Apr 4, 2022
@motoyugota
Copy link

I just wanted to add a note that this bug was actually fixed in this changeset: #6784

@michaelstaib michaelstaib removed the ❓ question This issue is a question about feature of Hot Chocolate. label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: EntityFramework Issue is related to the EntityFramework integration 🌶️ hot chocolate
Projects
None yet
Development

No branches or pull requests

6 participants