Consider revisiting EF.CompileQuery API

[divega]

Looking at the issue described at #14547, I realized that the way our current API plays with the type system can lead to some ambiguity and possibly confusion. For example:

• As described on Consider supporting more query operators inside EF.CompileQuery #14547, you can pass CompileAsyncQuery a non-async lambda that returns some TResult and that returns a Task. But this only work for a subset of the operators.
• On the other hand, according to the type system, it is ok to invoke the version of the method for non-async compiled queries, CompileQuery, but pass one of our async query operators, e.g. FirstAsync().
• Also, once you call any of the variations of the method that takes a lambda that returns an IQueryable, you commit prematurely to whether you will want to consume the results synchronously or asynchronously. This is strange given that CompileAsyncQuery and CompileQuery don't even perform any query compilation (compilation actually happens on first use).

I am not 100% sure what we should do, and if it is worth breaking the current API, but I see some opportunity to simplify it. One possibility is to remove the "CompileAsyncQuery" variation and return a CompiledQuery object from some/all overloads that contains methods to consume the results synchronously or asynchronously.

[ajcvickers]

@divega Would it be possible for you to bring an API proposal to the design meeting next week?

[ajcvickers]

Notes from the design meeting:

We will update the signature to be more like that of EF6. Specifically, using this pattern:

public static Func<TContext, TResult> CompileQuery<TContext, TResult>(
            [NotNull] Expression<Func<TContext, TResult>> queryExpression)

This aligns closely with the EF6 signatures, which will help with porting.

Note that TResult is not constrained. It may be:

• An IQueryable (e.g. normal result-set-returning queries, either async or not)
• Any type that can be returned as a single return object (e.g. .First() queries)
• A Task<> of this for single values used with async methods (e.g. .FirstAsync() queries)

This means that:

• For result-set queries, the async or not decision now depends on how the query is enumerated. There is no different API or different return type.
• For single-result queries, the return type is different (Task or not) but this isn't a big issue. When using the async version you get a Task to await on, which feels quite nice.

In EF6 it was possible to compose over a returned IQueryable, but it would mean that the query is executed normally, not as a compiled query. This can be a pit of failure. Decision from triage for EF Core is:

• Throw for now if an attempt is made to compose over the IQueryable
• Consider in the future turning this into a warning to allow composing (and breaking out of compiledness) if we get demand for it.

See also #15184 for considering returning ValueTask instead of Task.

[smitpatel]

Co-assigning to @divega so that we can discuss design in meeting when he is back from vacation.
While above API is written nicely but it is not possible to make it work statically easily.
Issues:

• When TResult is IQueryable type, we need to return IEnumerable/IAsyncEnumerable. On the flip side, when it is not, we need to return TResult/Task. There is not easy way to avoid confusion. Since if the same method has overload with IQueryable & just TResult then things work for most cases but when the expression ends with DbSet or Include (any type which implements IQueryable) then it picks up TResult overload. We cannot return DbSet object. So we need a way to distinguish both upto an extent (read the next point for further details)
• Since we want to return an object which can be enumerable sync/async both, we would need to have Execute/ExecuteAsync methods. If we want the static parameter binding check then we would have such an object for different number of parameters. I wanted to confirm that we want to do this before putting time in writing it out. If we decide that we don't want to do this and we will return Func only then we can possibly avoid issue in point 1.

I tried to look at EF6 code base to see how it worked.

• No async compiled queries at all.
• Returns func as @ajcvickers wrote in API above.
• Tests are making lambda expression explicitly returning IEnumerable rather than queryable.
• Works only with objectcontext.
• Query is converted to object query then cast to IEnumerable then cast to whatever result type is. Not sure how it all worked out when using IQueryable ending.

[ajcvickers]

Note that in final API review we decided not to change this for 3.0.

[smitpatel]

I am re-opening this to re-consider. (Given title, it was never closed-fixed, probably won't-fix)
Consider in association of #14547

[ajcvickers]

See also #13483. Specifically, pre-compiled queries assume the model for a given DbContext will never change.

[ajcvickers]

See discussion in #25368 about warnings when the expression tree is not appropriate; e.g. has ToList().

[ajcvickers]

Note from triage: we discussed this again and concluded that the cost and breaking nature of this change cannot be justified for the value it would bring.