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

[2.0/2.1] Query with manual groupjoin and another groupjoin/nav access in result selector based on different join predicate may return additional results #12200

Closed
yyjdelete opened this issue Jun 1, 2018 · 7 comments
Labels
area-query closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported type-bug

Comments

@yyjdelete
Copy link

yyjdelete commented Jun 1, 2018

Describe what is not working as expected.

GroupJoin get more data than outer in special case.

Steps to reproduce

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Generic;
using System.Linq;

namespace ConsoleApp15
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            using (var ctx = new Test_GroupJoinContext())
            {
                var needInit = false;
                if (needInit)
                {
                    #region init data
                    ctx.Database.EnsureDeleted();
                    ctx.Database.EnsureCreated();

                    var branchs = new List<Branch> { new Branch { Name = "B1" }, new Branch { Name = "B2" }, new Branch { Name = "B3" } };
                    ctx.Branch.AddRange(branchs);
                    var devices = new List<Device> { new Device { Name = "D1" }, new Device { Name = "D2" }, new Device { Name = "D3" }, new Device { Name = "D4" } };
                    ctx.Device.AddRange(devices);
                    ctx.SaveChanges();

                    branchs[0].MainDevice = devices[0];
                    branchs[1].MainDevice = devices[1];
                    branchs[2].MainDevice = devices[2];
                    devices[0].BranchNavigation = branchs[0];
                    devices[1].BranchNavigation = branchs[1];
                    devices[2].BranchNavigation = branchs[2];
                    devices[3].BranchNavigation = branchs[2];
                    ctx.SaveChanges();
                    #endregion
                }

                var tmp1 = ctx.Branch
                    .GroupJoin(ctx.Device, b => b.BranchId, d => d.BranchId, (b, ds) => new
                    {
                        b.BranchId,
                        MainDeviceName = b.MainDevice != null ? b.MainDevice.Name : null,
                        AllDevices = ds.ToList()
                    }).ToList();

                var tmp2 = ctx.Branch.Include(b => b.MainDevice).ToList()
                    .GroupJoin(ctx.Device.ToList(), b => b.BranchId, d => d.BranchId, (b, ds) => new
                    {
                        b.BranchId,
                        MainDeviceName = b.MainDevice != null ? b.MainDevice.Name : null,
                        AllDevices = ds.ToList()
                    }).ToList();

                //This happen with efcore2.1/2.0, but not with 1.1
                if (tmp1.Count != tmp2.Count)
                    Console.Error.WriteLine("Count of GroupJoin mismatch");
            }
            //Console.ReadLine();
        }
    }

    #region DbContext

    public partial class Test_GroupJoinContext : DbContext
    {
        public Test_GroupJoinContext()
        {
        }

        public Test_GroupJoinContext(DbContextOptions<Test_GroupJoinContext> options)
            : base(options)
        {
        }

        public virtual DbSet<Branch> Branch { get; set; }

        public virtual DbSet<Device> Device { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            if (!optionsBuilder.IsConfigured)
            {
                optionsBuilder.UseSqlServer("Server=(localdb)\\MSSQLLocalDB;Database=Test_GroupJoin;Trusted_Connection=True;");
            }

            var logger = new LoggerFactory();
            logger.AddConsole(LogLevel.Debug);
            optionsBuilder.UseLoggerFactory(logger);
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Branch>(entity =>
            {
                entity.HasOne(d => d.MainDevice)
                    .WithMany(p => p.Branch)
                    .HasForeignKey(d => d.MainDeviceId);
            });

            modelBuilder.Entity<Device>(entity =>
            {
                entity.HasOne(d => d.BranchNavigation)
                    .WithMany(p => p.Device)
                    .HasForeignKey(d => d.BranchId);
            });
        }
    }

    public partial class Device
    {
        public Device()
        {
            Branch = new HashSet<Branch>();
        }

        public int DeviceId { get; set; }

        public int? BranchId { get; set; }

        public string Name { get; set; }

        public Branch BranchNavigation { get; set; }

        public ICollection<Branch> Branch { get; set; }
    }

    public partial class Branch
    {
        public Branch()
        {
            Device = new HashSet<Device>();
        }

        public int BranchId { get; set; }

        public int? MainDeviceId { get; set; }

        public string Name { get; set; }

        public Device MainDevice { get; set; }

        public ICollection<Device> Device { get; set; }
    }

    #endregion DbContext
}

Expected 3 datas, and get 4. And if change MainDeviceName = b.MainDevice != null ? b.MainDevice.Name : null, to b.MainDevice, it works well. That happen with efcore2.1/2.0, but not in 1.1.

Further technical details

EF Core version: 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win10x64 10.0.17134.81
IDE: Visual Studio 2017 15.8prev2

@maumar
Copy link
Contributor

maumar commented Jun 11, 2018

Problem here is that both GroupJoin and b.MainDevice.* translates to LEFT JOIN which are based on different join conditions. The query we generate looks like this:

SELECT [b].[BranchId] AS [BranchId0], [b].[MainDeviceId], [b].[Name], [d].[DeviceId], [d].[BranchId], [d].[Name], [b.MainDevice].[Name]
FROM [Branch] AS [b]
LEFT JOIN [Device] AS [b.MainDevice] ON [b].[MainDeviceId] = [b.MainDevice].[DeviceId]
LEFT JOIN [Device] AS [d] ON [b].[BranchId] = [d].[BranchId]
ORDER BY [BranchId0]

What we should probably do is detect manually generated groupjoins, and any other groupjoins inside that groupjoin's result selector that are based on different keys should be translated to the subquery instead of LOJ.

Optional navigation -> LOJ translation was introduced in 2.x

@maumar maumar removed this from the 2.2.0 milestone Jun 11, 2018
@maumar maumar changed the title [2.0/2.1]GroupJoin get more data than outer with special code in db [2.0/2.1] Query with manual groupjoin and another groupjoin/nav access in result selector based on different join predicate may return additional results Jun 11, 2018
@ajcvickers ajcvickers added this to the 2.2.0 milestone Jun 12, 2018
@maumar
Copy link
Contributor

maumar commented Aug 30, 2018

We should translate to something like this:

from b in ctx.Branch
join d in ctx.Device on b.BranchId equals d.BranchId into ds
 select new
 {
     b.BranchId,
     Device = b.MainDevice != null ? (from md in ctx.Device
                                      where md.DeviceId == b.MainDeviceId
                                      select md.Name).FirstOrDefault() : null,
     List = ds.ToList()
 };

@ajcvickers ajcvickers modified the milestones: 2.2.0-preview2, 2.2.0 Sep 11, 2018
@maumar
Copy link
Contributor

maumar commented Oct 2, 2018

Simpler repro:

                var query = from b in ctx.Branch
                            join d in ctx.Device on b.BranchId equals d.BranchId into grouping
                            select b.MainDevice != null ? b.MainDevice.Name : null;

                var result = query.ToList();

@maumar
Copy link
Contributor

maumar commented Oct 4, 2018

Generated query plan for the above case:

(QueryContext queryContext) => IEnumerable<string> _InterceptExceptions(
|__ source: IEnumerable<string> _Select(
|   |__ source: IEnumerable<TransparentIdentifier<TransparentIdentifier<Branch, ValueBuffer>, IEnumerable<Device>>> _GroupJoin(
|   |   |__ queryContext: (RelationalQueryContext)queryContext, 
|   |   |__ source: IEnumerable<ValueBuffer> _Query(
|   |   |   |__ queryContext: queryContext, 
|   |   |   |__ shaperCommandContext: SelectExpression: 
|   |   |           SELECT [b].[BranchId], [b].[MainDeviceId], [b].[Name], [d].[DeviceId], [d].[BranchId], [d].[Name], [b.MainDevice].[Name]
|   |   |           FROM [Branch] AS [b]
|   |   |           LEFT JOIN [Device] AS [b.MainDevice] ON [b].[MainDeviceId] = [b.MainDevice].[DeviceId]
|   |   |           LEFT JOIN [Device] AS [d] ON [b].[BranchId] = [d].[BranchId]
|   |   |           ORDER BY [b].[BranchId]), 
|   |   |__ outerShaper: (Branch b | ValueBuffer b.MainDevice) => TransparentIdentifier<Branch, ValueBuffer> CreateTransparentIdentifier(
|   |   |   |__ outer: b, 
|   |   |   |__ inner: b.MainDevice), 
|   |   |__ innerShaper: BufferedOffsetEntityShaper<Device>, 
|   |   |__ innerKeySelector: (Device d) => d.BranchId, 
|   |   |__ resultSelector: (TransparentIdentifier<Branch, ValueBuffer> t1 | IEnumerable<Device> grouping) => TransparentIdentifier<TransparentIdentifier<Branch, ValueBuffer>, IEnumerable<Device>> CreateTransparentIdentifier(
|   |       |__ outer: t1, 
|   |       |__ inner: grouping)), 
|   |__ selector: (TransparentIdentifier<TransparentIdentifier<Branch, ValueBuffer>, IEnumerable<Device>> t2) => !(t2.Outer.Inner.IsEmpty) ? !(t2.Outer.Inner.IsEmpty) ? string TryReadValue(t2.Outer.Inner, 6, Device.Name) : default(string) : default(string)), 
|__ contextType: Repro12200.MyContext, 
|__ logger: DiagnosticsLogger<Query>, 
|__ queryContext: queryContext)

Problem here is that outer shaper for client GroupJoin is TransparentIdentifier<Branch, ValueBuffer> - this is because navigation expansion that happens "inside" / "before" group join.

When we process client group join, trying to place inners into correct buckets we check if the outers are the same (keep adding to the bucket) or different (need to open another bucket). However, in the case above, outer will differ even when the Branch is the same - this is because outer is a composite of Branch and value buffer.

We should detect where client groupjoins are present to the query and for those cases translate all related navs into subqueries, rather than joins.

@maumar maumar removed this from the 2.2.0 milestone Oct 4, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Oct 5, 2018
@maumar
Copy link
Contributor

maumar commented Jun 19, 2019

currently this fails in new nav expansion pipeline:

Field 'Microsoft.EntityFrameworkCore.Query.NavigationExpansion.TransparentIdentifier`2[Microsoft.EntityFrameworkCore.Query.QueryBugsTest+Branch,System.Collections.Generic.IEnumerable`1[Microsoft.EntityFrameworkCore.Query.QueryBugsTest+Device]].Inner' is not defined for type 'Microsoft.EntityFrameworkCore.Query.NavigationExpansion.TransparentIdentifier`2[Microsoft.EntityFrameworkCore.Query.NavigationExpansion.TransparentIdentifier`2[Microsoft.EntityFrameworkCore.Query.QueryBugsTest+Branch,System.Collections.Generic.IEnumerable`1[Microsoft.EntityFrameworkCore.Query.QueryBugsTest+Device]],Microsoft.EntityFrameworkCore.Query.QueryBugsTest+Device]'

   at System.Linq.Expressions.Expression.Field(Expression expression, FieldInfo field)
   at System.Linq.Expressions.Expression.MakeMemberAccess(Expression expression, MemberInfo member)
   at System.Linq.Expressions.MemberExpression.Update(Expression expression)
   at System.Linq.Expressions.ExpressionVisitor.VisitMember(MemberExpression node)
   at System.Linq.Expressions.MemberExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.ExpressionReplacingVisitor.Visit(Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\ExpressionReplacingVisitor.cs:line 43
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.ExpressionReplacingVisitor.Visit(Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\ExpressionReplacingVisitor.cs:line 43
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.NavigationExpansionExpression.VisitChildren(ExpressionVisitor visitor) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\NavigationExpansionExpression.cs:line 38
   at System.Linq.Expressions.ExpressionVisitor.VisitExtension(Expression node)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.ExpressionReplacingVisitor.Visit(Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\ExpressionReplacingVisitor.cs:line 43
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitNew(NewExpression node)
   at System.Linq.Expressions.NewExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.ExpressionReplacingVisitor.Visit(Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\ExpressionReplacingVisitor.cs:line 43
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.NavigationExpandingVisitor.FindAndApplyNavigations(Expression source, LambdaExpression lambda, NavigationExpansionExpressionState state) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\NavigationExpandingVisitor_MethodCall.cs:line 1269
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.NavigationExpandingVisitor.ProcessGroupJoin(MethodCallExpression methodCallExpression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\NavigationExpandingVisitor_MethodCall.cs:line 667
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.Visitors.NavigationExpandingVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\Visitors\NavigationExpandingVisitor_MethodCall.cs:line 56
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.NavigationExpansion.NavigationExpander.ExpandNavigations(Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\NavigationExpansion\NavigationExpander.cs:line 25
   at Microsoft.EntityFrameworkCore.Query.Pipeline.QueryOptimizer.Visit(Expression query) in D:\git\EntityFrameworkCore\src\EFCore\Query\Pipeline\QueryOptimizingExpressionVisitor.cs:line 26
   at Microsoft.EntityFrameworkCore.Query.Pipeline.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query) in D:\git\EntityFrameworkCore\src\EFCore\Query\Pipeline\QueryCompilationContext2.cs:line 59
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async) in D:\git\EntityFrameworkCore\src\EFCore\Storage\Database.cs:line 72
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async) in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\QueryCompiler.cs:line 108
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0() in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\QueryCompiler.cs:line 97
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler) in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\CompiledQueryCache.cs:line 84
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler) in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\CompiledQueryCache.cs:line 59
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\QueryCompiler.cs:line 93
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression) in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\EntityQueryProvider.cs:line 79
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator() in D:\git\EntityFrameworkCore\src\EFCore\Query\Internal\EntityQueryable`.cs:line 94
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)


@smitpatel
Copy link
Contributor

Blocked on #17068

@smitpatel
Copy link
Contributor

As per #17068, we don't process GroupJoin since it cannot be translated to server. Hence above queries won't work post 3.0. Closing as not needed.

@smitpatel smitpatel added closed-not-needed closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed punted-for-3.0 closed-not-needed labels Nov 22, 2019
@smitpatel smitpatel removed this from the Backlog milestone Nov 22, 2019
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported type-bug
Projects
None yet
Development

No branches or pull requests

4 participants