Query: Throw better exception when aggregate operation hits client eval

[smitpatel]

No description provided.

[ajcvickers]

Going to work on this.

[ajcvickers]

@divega Message proposal:

The LINQ expression '{expression}' could not be translated. Either rewrite the query in a form that can be translated, or use AsEnumerable(), ToList(), or similar to explicitly bring data back to the client where it can then be further processed using LINQ-to-Objects. See fwlink for more information.

Feedback? And fwlink...

[smitpatel]

I would suggest using AsEnumerable/AsAsyncEnumerable. ToList also works but creates object which may not be needed. And ToListAsync would involve await keyword. Those patterns could go to fwlink.

[divega]

I would also wish we could just suggest AsEnumerable and AsAsyncEnumerable, but the problem with the latter is that any further LINQ query composition on the client requires adding System.Linq.Async as a dependency to the application. So in that sense ToListAsync() can be a simpler thing to propose, even if it requires awaiting before doing any in-memory proccesing.

FWIW, in the latest version of the LINQ workarounds that I included in the Preview 8 blog post I thought about this and I ended up deciding to mention all 4 methods.

LINQ to Objects doesn't have hyphens.

Re the fwlink, it is less than ideal, but given that we don't have versioning in our documentation perhaps we should keep the same fwlink we had in the warning for client evaluation before (https://go.microsoft.com/fwlink/?linkid=2097910), which points to https://docs.microsoft.com/ef/core/querying/client-eval, and update the article to add a section for EF Core 3.0 and move the current content to a section about previous versions.

[ajcvickers]

@divega @smitpatel Based on the blog text:

The LINQ expression '{expression}' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync(). See https://go.microsoft.com/fwlink/?linkid=2097910 for more information.

I agree with leaving ToList/ToListAsync here. It's something easy and simple that people can grok. The doc page can go into the trade-offs of the different approaches.

[divega]

I changed my mind about the fwlink. It is more future-proof to create a new one for 3.0, even if for now they point to the same page: https://go.microsoft.com/fwlink/?linkid=2101038.

The doc page can go into the trade-offs of the different approaches.

Good point. I created dotnet/EntityFramework.Docs#1648 to track this.

[ajcvickers]

Some tests still throwing a bad message after initial work here:

Microsoft.EntityFrameworkCore.Query.SimpleQuerySqlServerTest.Average_over_max_subquery_is_client_eval(isAsync: False)

Microsoft.Data.SqlClient.SqlException : Cannot perform an aggregate function on an expression containing an aggregate or a subquery.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject) in C:\aspnet\EntityFrameworkCore\src\EFCore.Relational\Storage\RelationalCommand.cs:line 400
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.QueryingEnumerable`1.Enumerator.MoveNext() in C:\aspnet\EntityFrameworkCore\src\EFCore.Relational\Query\RelationalShapedQueryCompilingExpressionVisitor.QueryingEnumerable.cs:line 96
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) in C:\aspnet\EntityFrameworkCore\src\EFCore\Query\Internal\QueryCompiler.cs:line 98
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression) in C:\aspnet\EntityFrameworkCore\src\EFCore\Query\Internal\EntityQueryProvider.cs:line 79
   at System.Linq.Queryable.Average[TSource](IQueryable`1 source, Expression`1 selector)
   at Microsoft.EntityFrameworkCore.TestUtilities.QueryAsserter`1.AssertAverage[TItem1,TSelector](Func`2 actualQuery, Func`2 expectedQuery, Expression`1 actualSelector, Expression`1 expectedSelector, Action`2 asserter, Boolean isAsync) in C:\aspnet\EntityFrameworkCore\test\EFCore.Specification.Tests\TestUtilities\QueryAsserter.cs:line 2437
   at Microsoft.EntityFrameworkCore.Query.SimpleQuerySqlServerTest.Average_over_max_subquery_is_client_eval(Boolean isAsync) in C:\aspnet\EntityFrameworkCore\test\EFCore.SqlServer.FunctionalTests\Query\SimpleQuerySqlServerTest.ResultOperators.cs:line 235
--- End of stack trace from previous location where exception was thrown ---

[smitpatel]

SqlException since it is invalid SQL. I suggest, verify as SqlException only. In past we identified such scenario and did client eval. No point of doing work of identifying this pattern.

[ajcvickers]

Putting this on the backlog since not generating SQL (instead throwing early) that won't work is better than letting it throw, but this isn't high priority. (Until feedback says it is.. ;-))

[kierenj]

I have some similarly invalid SQL generated via trying a .Sum(x => x.SomeCol - x.OtherCol). I wouldn't mind the option of having it run client-side. Anything I can do?

[roji]

Am going to go ahead and close this. @ajcvickers already improved the exception message for client eval (#17247); regarding "Cannot perform an aggregate function on an expression containing an aggregate or a subquery" specifically (which is a SQL Server limitation error), we should try to rewrite the query to work (#34256), rather than trying to throw a better exception message; identifying the specific case where the SQL Server error would be produced would be half-way towards fixing the issue in any case.