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

Group by an empty key with an aggregate throws 'must be reducible node' #11905

Closed
AlekseyMartynov opened this issue May 4, 2018 · 6 comments
Closed
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. regression type-bug
Milestone

Comments

@AlekseyMartynov
Copy link

Version: 2.1.0-preview2-final

Query:

var context = new NorthwindContext();

var query = context.Orders
    .GroupBy(obj => new { }) // ← here
    .Select(g => new {
        Sum = g.Sum(obj => obj.Freight)
    });

Exception:

System.ArgumentException: must be reducible node
   at System.Linq.Expressions.Expression.ReduceAndCheck()
   at System.Linq.Expressions.Expression.ReduceExtensions()
   
   ... repeated System.Linq.Expressions.Compiler.StackSpiller lines ...

   at System.Linq.Expressions.Expression`1.Accept(StackSpiller spiller)
   at System.Linq.Expressions.Compiler.LambdaCompiler.Compile(LambdaExpression lambda)
   at System.Linq.Expressions.Expression`1.Compile(Boolean preferInterpretation)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateExecutorLambda[TResults]()
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](Expression query, IQueryModelGenerator queryModelGenerator, IDatabase database, IDiagnosticsLogger`1 logger, Type contextType)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass13_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Remotion.Linq.QueryableBase`1.System.Collections.IEnumerable.GetEnumerator()

EF6 seems to handle this query by rewriting to .GroupBy(obj => 1):

SELECT 
    1 AS [C1], 
    [GroupBy1].[A1] AS [C2]
    FROM ( SELECT 
        [Extent1].[K1] AS [K1], 
        SUM([Extent1].[A1]) AS [A1]
        FROM ( SELECT 
            cast(1 as bit) AS [K1], 
            [Extent1].[Freight] AS [A1]
            FROM [dbo].[Orders] AS [Extent1]
        )  AS [Extent1]
        GROUP BY [K1]
    )  AS [GroupBy1]
@AlekseyMartynov
Copy link
Author

Technically, this is a regression. Versions 2.0.2 and 2.1.0-preview1-final execute this query in memory and don't throw.

@smitpatel
Copy link
Contributor

@divega @ajcvickers - Do we want to block this for 2.1 or convert to group by constant?

@divega
Copy link
Contributor

divega commented May 4, 2018

@AlekseyMartynov what is the scenario exactly for this? I am guessing that a DevExpress component uses this expression, but please confirm.

@smitpatel smitpatel added this to the 2.1.0 milestone May 4, 2018
@smitpatel smitpatel self-assigned this May 4, 2018
smitpatel added a commit that referenced this issue May 4, 2018
Resolves #11905

Issue: GroupBy(g => new {}) converts to GroupBy(Constant) because of key not being correlated with range variable.
We translate GroupBy constant to server but in this case, the constant is of anon type which cannot be put in SQL tree hence translation pipeline failed.
Fix is to inject `1` as constant whenever we see GroupBy constant of a type which cannot be represented in SQL
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 4, 2018
@smitpatel
Copy link
Contributor

@AlekseyMartynov - To add a bit more, with the fix, we will inject a custom Key of 1 instead of anonymous type. So if the key is used anywhere then it may have different result from l2o.
Further while this was doing client eval in 2.0, it was basically fetching whole table to client which had bad performance regardless. Hence we decided to inject the constant.

smitpatel added a commit that referenced this issue May 4, 2018
Resolves #11905

Issue: GroupBy(g => new {}) converts to GroupBy(Constant) because of key not being correlated with range variable.
We translate GroupBy constant to server but in this case, the constant is of anon type which cannot be put in SQL tree hence translation pipeline failed.
Fix is to inject `1` as constant whenever we see GroupBy constant of a type which cannot be represented in SQL.
We also don't save it in mapping so that if key is projected out, we construct it again on client side.
@AlekseyMartynov
Copy link
Author

@divega:

what is the scenario exactly for this? I am guessing that a DevExpress component uses this expression, but please confirm.

Correct. I mentioned this case in #2341 (comment).

@smitpatel:
For our needs, both .GroupBy(obj => new { }) and .GroupBy(obj => C) would work. We have used the former as a degenerate case of grouping, when all data is treated as one group. The fix in EF Core will help maintain compatibility with existing verisons of our library. Thank you.

@smitpatel
Copy link
Contributor

@AlekseyMartynov - With the fix going in, we implemented that, if you do GroupBy(obj => new { }) we will generate GroupBy 1 on server. At the same time if you are selecting grouping.Key on client, we will cause client eval of the projection partially to generate the Key on client but all other aggregate should be translated to server. Hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. regression type-bug
Projects
None yet
Development

No branches or pull requests

4 participants