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

Must be reductible node #11933

Closed
NicolasDorier opened this issue May 8, 2018 · 32 comments · Fixed by #17805
Closed

Must be reductible node #11933

NicolasDorier opened this issue May 8, 2018 · 32 comments · Fixed by #17805
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

@NicolasDorier
Copy link

NicolasDorier commented May 8, 2018

Describe what is not working as expected.

I am migrating to .NET 2.1, but it seems entity framework broke:
This was working in 2.0:

var userId = GetUserId();
using (var ctx = _ContextFactory.CreateContext())
{
    return await ctx.UserStore
        .Where(us => us.ApplicationUserId == userId)
        .SelectMany(us => us.StoreData.Apps.Select(app => new ListAppsViewModel.ListAppViewModel()
        {
            IsOwner = us.Role == StoreRoles.Owner,
            StoreId = us.StoreDataId,
            StoreName = us.Role,
            AppName = app.Name,
            AppType = app.AppType,
            Id = app.Id
        }))
        .ToArrayAsync();

Now it throws

at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at BTCPayServer.Tests.UnitTest1.CanUsePoSApp() in C:\Sources\btcpayserver\BTCPayServer.Tests\UnitTest1.cs:line 1180
----- Inner Stack Trace -----
   at System.Linq.Expressions.Expression.ReduceAndCheck()
   at System.Linq.Expressions.Expression.ReduceExtensions()
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExtensionExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMemberExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.MemberAssignmentRewriter..ctor(MemberAssignment binding, StackSpiller spiller, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.BindingRewriter.Create(MemberBinding binding, StackSpiller spiller, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMemberInitExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpressionFreeTemps(Expression expression, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.Rewrite[T](Expression`1 lambda)
   at System.Linq.Expressions.Expression`1.Accept(StackSpiller spiller)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteLambdaExpression(Expression expr)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.Add(Expression expression)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.AddArguments(IArgumentProvider expressions)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMethodCallExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpressionFreeTemps(Expression expression, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.Rewrite[T](Expression`1 lambda)
   at System.Linq.Expressions.Expression`1.Accept(StackSpiller spiller)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteLambdaExpression(Expression expr)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.Add(Expression expression)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.AddArguments(IArgumentProvider expressions)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMethodCallExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.Add(Expression expression)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.AddArguments(IArgumentProvider expressions)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMethodCallExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.Add(Expression expression)
   at System.Linq.Expressions.Compiler.StackSpiller.ChildRewriter.AddArguments(IArgumentProvider expressions)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteMethodCallExpression(Expression expr, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpression(Expression node, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.RewriteExpressionFreeTemps(Expression expression, Stack stack)
   at System.Linq.Expressions.Compiler.StackSpiller.Rewrite[T](Expression`1 lambda)
   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.CreateAsyncQueryExecutor[TResult](QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.System.Collections.Generic.IAsyncEnumerable<TResult>.GetEnumerator()
   at System.Collections.Generic.AsyncEnumerableHelpers.ToArrayWithLength[T](IAsyncEnumerable`1 source, CancellationToken cancellationToken)
   at System.Collections.Generic.AsyncEnumerableHelpers.ToArray[T](IAsyncEnumerable`1 source, CancellationToken cancellationToken)
   at BTCPayServer.Controllers.AppsController.GetAllApps() in C:\Sources\btcpayserver\BTCPayServer\Controllers\AppsController.cs:line 178
   at BTCPayServer.Controllers.AppsController.ListApps() in C:\Sources\btcpayserver\BTCPayServer\Controllers\AppsController.cs:line 44

Note that if I comment

            StoreId = us.StoreDataId,
            StoreName = us.Role,

it does not crash.

<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="2.0.2" />
<PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0-rc1-final" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version=" 2.1.0-rc1-final" PrivateAssets="All" />

Workaround:

return await ctx.UserStore
    .Where(us => us.ApplicationUserId == userId)
    .Join(ctx.Apps, us => us.StoreDataId, app => app.StoreDataId,
    (us, app) =>
    new ListAppsViewModel.ListAppViewModel()
    {
        IsOwner = us.Role == StoreRoles.Owner,
        StoreId = us.StoreDataId,
        StoreName = us.StoreData.StoreName,
        AppName = app.Name,
        AppType = app.AppType,
        Id = app.Id
    })
    .ToArrayAsync();
@roji
Copy link
Member

roji commented May 8, 2018

Just to get more data, would it be possible to test with 2.1.0-preview2-final for all packages (both Microsoft and Npgsql)? This would tell us whether is coming from mixing 2.0 and 2.1

(Npgsql's rc1 packages aren't out yet, but that should happen very soon)

@NicolasDorier
Copy link
Author

sure one moment

@NicolasDorier
Copy link
Author

NicolasDorier commented May 8, 2018

Same issue in:

  • 2.1.0-preview2-final
  • 2.1.0-preview1-final

@NicolasDorier
Copy link
Author

Note I did not updated Npgsql

@roji
Copy link
Member

roji commented May 8, 2018

Note I did not updated Npgsql

@NicolasDorier as I wrote above the idea is to align both Microsoft and Npgsql versions... Can you please try with all packages on 2.1.0-preview2, both Microsoft and Npgsql?

@NicolasDorier
Copy link
Author

Ah sorry, I missed the preview package for Postgres as they were not listed by the nuget UI, trying now.

@NicolasDorier
Copy link
Author

NicolasDorier commented May 8, 2018

Both 2.1.0-preview2, same issue. It is interesting that postgres 2.0.2 works with 2.0.1rc, but postgres 2.1preview does not work with 2.0.1rc. (unrelated error)

@bmarder
Copy link

bmarder commented May 8, 2018

I received this error frequently with certain queries as well, using sqlserver and 2.1 preview 2. @NicolasDorier Thanks for giving a full repo for this issue.

@ajcvickers
Copy link
Contributor

@maumar Can you investigate this with regard to it being a regression in 2.1?

@maumar
Copy link
Contributor

maumar commented May 8, 2018

@ajcvickers verified this is a regression - the code was working fine in 2.0.2

@maumar
Copy link
Contributor

maumar commented May 8, 2018

query plan (with non-reducible nodes highlighted):

(QueryContext queryContext) => IAsyncEnumerable<ListAppViewModel> _InterceptExceptions(
    source: IAsyncEnumerable<ListAppViewModel> _Select(
        source: IAsyncEnumerable<TransparentIdentifier<TransparentIdentifier<ValueBuffer, ValueBuffer>, ListAppViewModel>> _SelectMany(
            source: IAsyncEnumerable<TransparentIdentifier<ValueBuffer, ValueBuffer>> _ShapedQuery(
                queryContext: queryContext, 
                shaperCommandContext: SelectExpression: 
                    SELECT [us.StoreData].[Id], [us].[Role], [us].[StoreDataId]
                    FROM [UserStore] AS [us]
                    INNER JOIN [StoreData] AS [us.StoreData] ON [us].[StoreDataId] = [us.StoreData].[Id]
                    WHERE [us].[ApplicationUserId] = @__userId_0, 
                shaper: TypedCompositeShaper<ValueBufferShaper, ValueBuffer, ValueBufferShaper, ValueBuffer, TransparentIdentifier<ValueBuffer, ValueBuffer>>), 
            collectionSelector: (TransparentIdentifier<ValueBuffer, ValueBuffer> t0) => IAsyncEnumerable<ListAppViewModel> _Select(
                source: IAsyncEnumerable<ValueBuffer> _ShapedQuery(
                    queryContext: queryContext, 
                    shaperCommandContext: SelectExpression: 
                        SELECT [t].[IsOwner], [t].[StoreId], [t].[StoreName], [t].[AppName], [t].[AppType], [t].[Id]
                        FROM (
                            SELECT CASE
                                WHEN [us].[Role] = 1
                                THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
                            END AS [IsOwner], [us].[StoreDataId] AS [StoreId], [us].[Role] AS [StoreName], [app].[Name] AS [AppName], [app].[AppType], [app].[Id]
                            FROM [App] AS [app]
                            WHERE [us.StoreData].[Id] = [app].[StoreDataId]
                        ) AS [t], 
                    shaper: ValueBufferShaper), 
                selector: (ValueBuffer app) => new ListAppViewModel{ 
                    IsOwner = (bool)Nullable<bool> TryReadValue(app, 0, null), 
                    StoreId =  ---> [us] <--- .StoreDataId, 
                    StoreName =  ---> [us] <--- .Role, 
                    AppName = string TryReadValue(app, 3, App.Name), 
                    AppType = AppType TryReadValue(app, 4, App.AppType), 
                    Id = int TryReadValue(app, 5, App.Id) 
                }
            ), 
            resultSelector: (TransparentIdentifier<ValueBuffer, ValueBuffer> t0 | ListAppViewModel <generated>_0) => TransparentIdentifier<TransparentIdentifier<ValueBuffer, ValueBuffer>, ListAppViewModel> CreateTransparentIdentifier(
                outer: t0, 
                inner: <generated>_0)), 
        selector: (TransparentIdentifier<TransparentIdentifier<ValueBuffer, ValueBuffer>, ListAppViewModel> t1) => t1.Inner), 
    contextType: Repro11933.MyContext, 
    logger: DiagnosticsLogger<Query>, 
    queryContext: queryContext)

@maumar
Copy link
Contributor

maumar commented May 8, 2018

query plan in 2.0.2 - SelectMany gets translated, in current bits it gets client-evaled

Microsoft.EntityFrameworkCore.Query[10107]
      (QueryContext queryContext) => IAsyncEnumerable<ListAppViewModel> _InterceptExceptions(
          source: IAsyncEnumerable<ListAppViewModel> _ShapedQuery(
              queryContext: queryContext,
              shaperCommandContext: SelectExpression:
                  SELECT CASE
                      WHEN [us].[Role] = 1
                      THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
                  END AS [IsOwner], [us].[StoreDataId] AS [StoreId], [us].[Role] AS [StoreName], [us.StoreData.Apps].[Name] AS [AppName], [us.StoreData.Apps].[AppType], [us.StoreData.Apps].[Id]
                  FROM [UserStore] AS [us]
                  INNER JOIN [StoreData] AS [us.StoreData] ON [us].[StoreDataId] = [us.StoreData].[Id]
                  INNER JOIN [App] AS [us.StoreData.Apps] ON [us.StoreData].[Id] = [us.StoreData.Apps].[StoreDataId]
                  WHERE [us].[ApplicationUserId] = @__userId_0,
              shaper: TypedProjectionShaper<TypedCompositeShaper<TypedCompositeShaper<ValueBufferShaper, ValueBuffer, ValueBufferShaper, ValueBuffer, TransparentIdentifier<ValueBuffer, ValueBuffer>>, TransparentIdentifier<ValueBuffer, ValueBuffer>, ValueBufferShaper, ValueBuffer, TransparentIdentifier<TransparentIdentifier<ValueBuffer, ValueBuffer>, ValueBuffer>>, TransparentIdentifier<TransparentIdentifier<ValueBuffer, ValueBuffer>, ValueBuffer>, ListAppViewModel>),
          contextType: EFSampleApp.MyContext,
          logger: DiagnosticsLogger<Query>,
          queryContext: queryContext)

@ajcvickers
Copy link
Contributor

@maumar Do you need @anpete or @smitpatel to help on this?
/cc @divega

@maumar
Copy link
Contributor

maumar commented May 8, 2018

@ajcvickers the regression comes from nav rewrite, so I'm the right person to work on it.

@maumar
Copy link
Contributor

maumar commented May 8, 2018

simplified repro:

    class Program
    {
        static void Main(string[] args)
        {
            using (var ctx = new MyContext())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();
            }

            using (var ctx = new MyContext())
            {
                var query = ctx.Customers.SelectMany(c => c.Orders.Select(o => new { OrderName = o.Name, CustomerName = c.Name }));
                var result = query.ToList();
            }
        }
    }

    public class Customer
    {
        public int Id { get; set; }
        public string Name { get; set; }

        public List<Order> Orders { get; set; }
    }

    public class Order
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }

    public class MyContext : DbContext
    {
        public DbSet<Customer> Customers { get; set; }
        public DbSet<Order> Orders { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=.;Database=Repro11933;Trusted_Connection=True;MultipleActiveResultSets=True");
        }
    }

@maumar
Copy link
Contributor

maumar commented May 8, 2018

Error happens because we changed translation of SelectMany for non-qsre subqueries - #11219 Now we translate those into subqueries, rather than joins. This however exposes the binding issue, when the subquery has references to the outer query sources.

Given that this query used to work in 2.0.2 the fix to #11219 was incorrect (or at least too aggressive).

@ajcvickers
Copy link
Contributor

Triage: Plan is currently to fix #11941, which should also fix this.
@maumar to investigate if 2.1 will client eval more cross apply cases than before.

@btecu
Copy link
Contributor

btecu commented Aug 12, 2019

This is a big blocker for us and we've been waiting for EF Core 3.0. Is it not going to be fixed in 3.0 anymore?

@maumar
Copy link
Contributor

maumar commented Aug 12, 2019

@btecu SelectMany translation is being implemented at the moment (you can track the PR here: #17077). Entire query pipeline has been rewritten for 3.0 so once SelectMany support is in, this bug may simply go away.

@AndriySvyryd AndriySvyryd added verify-fixed This issue is likely fixed in new query pipeline. and removed blocked labels Aug 22, 2019
@btecu
Copy link
Contributor

btecu commented Aug 29, 2019

Can we verify this is fixed?

@ajcvickers
Copy link
Contributor

@btecu Yes. You can use the daily builds to do your verification.

@ajcvickers ajcvickers modified the milestones: Backlog, 3.1.0 Sep 4, 2019
@maumar maumar removed punted-for-3.0 verify-fixed This issue is likely fixed in new query pipeline. labels Sep 11, 2019
@maumar maumar modified the milestones: 3.1.0, 3.0.0 Sep 11, 2019
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 11, 2019
@maumar
Copy link
Contributor

maumar commented Sep 11, 2019

verified this issue has been fixed in 3.0

@smitpatel
Copy link
Contributor

@maumar - Regression test.

@maumar
Copy link
Contributor

maumar commented Sep 11, 2019

@smitpatel i have test ready, will send PR with regressions for multiple issues

@btecu
Copy link
Contributor

btecu commented Sep 11, 2019

@ajcvickers I tested preview 9 and confirm that previous query that in 2.x was broken now works.

maumar added a commit that referenced this issue Sep 12, 2019
Resolves #8723
Resolves #9241
Resolves #10172
Resolves #10210
Resolves #10548
Resolves #11847
Resolves #11933
Resolves #12741
Resolves #15798
@maumar maumar closed this as completed in 36a7bdf Sep 12, 2019
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

Successfully merging a pull request may close this issue.

10 participants