EF Core 1.1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model #7944

shaulbehr · 2017-03-21T17:54:38Z

I saw today that EF Core 1.1 has been released. I made a new branch and upgraded from 1.0. Now several of our integration tests are failing with the below error. I'm not going to attempt to give you the full code base that raises this error; our codebase is massive. Rather I will just paste the line of code that throws the error along with the stack trace and hope this will be helpful to you. In the meantime I'm abandoning my attempt to upgrade until I hear back that this bug has been repaired.

The line of code that raises the exception:

                            var result = pagedResult
                                .Select(bt => new LibraryManagerSubShelf
                                {
                                    TitleStartingLetter = bt.titleStartingLetter.ToUpper()
                                })
                                .ToList();

where pagedResult is an IQueryable<> that has had a .Skip().Take() applied to it. I verified that if the skip+take is removed, the exception is not thrown.

Stack trace:

System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryModelExtensions.QueryModelMappingPopulatingVisitor.VisitSubQuery(SubQueryExpression expression)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitAndConvert[T](ReadOnlyCollection`1 nodes, String callerName)
   at Remotion.Linq.Parsing.RelinqExpressionVisitor.VisitNew(NewExpression expression)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.Visit(Expression node)
   at Remotion.Linq.Clauses.SelectClause.TransformExpressions(Func`2 transformation)
   at Remotion.Linq.QueryModel.TransformExpressions(Func`2 transformation)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryModelExtensions.QueryModelMappingPopulatingVisitor.VisitSubQuery(SubQueryExpression expression)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.ExpressionVisitorBase.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryModelExtensions.PopulateQueryModelMapping(QueryModel queryModel, Dictionary`2 mapping)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.LiftSubQuery(IQuerySource querySource, Expression itemsExpression, Expression expression)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitMainFromClause(MainFromClause fromClause, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.Internal.SqlServerQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   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, INodeTypeProvider nodeTypeProvider, IDatabase database, ILogger logger, Type contextType)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass19_0`1.<CompileQuery>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.GetEnumerator()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at MyCode.MyRepository.<>c__DisplayClass1_0.<GetDistinctLibraryManagerShelves>b__0(MyDbContext db) in C:\src\MyCode\MyRepository.cs:line 63

The text was updated successfully, but these errors were encountered:

smitpatel · 2017-03-21T21:20:39Z

@maumar - Did you investigate this?

maumar · 2017-03-21T21:58:17Z

In QueryModelMappingPopulatingVisitor before adding model to the dictionary we should check if it's already been added. This should be safe to do, but will do some more testing to make sure.

smitpatel · 2017-03-21T22:01:03Z

While having defensive add to dictionary is safe to do, we should also investigate the reason for it to arise and evaluate if we want to preserve the first value or last value. Also if such cases should be impossible (but happening due to bug somewhere else) then defensive check would make it harder for us to catch the issue.

maumar · 2017-03-22T19:33:06Z

@shaulbehr are there other operations applied on the IQueryable in the pagedResult (filters, orderbys etc) or is it just entities.Skip(..).Take(...)?

maumar · 2017-03-22T21:12:13Z

standalone repro:

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

                var innerQuery = from c in ctx.Customers
                            let customers = ctx.Customers.Select(cc => cc)
                            where customers.Any()
                            select customers;

                var outerQuery = ctx.Customers.Where(c => innerQuery.Any()).ToList();
            }
        }
    }

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

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

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

…or complex queries with with subqueries, if the same subquery is present multiple times in the query model Problem was that when we fixed #7476 we introduced a mechanism to copy and then restore query models to prevent them from being corrupted during (failed) attempt at translating them to SQL. However we have not accounted for the case where the same subquery would be present in the query model more than once (same reference). Fix is to add defensive check to the logic that creates a copy of a query model. If we already copied that particular (sub)query model, we don't need to do it twice. It is safe to do, because all occurrences of the query model that share the same reference are guaranteed to have the same body clauses and therefore can later be re-created from the same copy.

maumar · 2017-03-23T01:24:12Z

Fixed in 0594995

maumar · 2017-03-23T19:55:33Z

Justification to include this in 1.1.2 release:

Impact: Medium. This is a regression introduced by patch 1.1.1 and was reported by a customer. However the scenario affected is quite complex - it requires a subquery nested within another subquery.

Risk: Low. Fix is very local - basically we are adding defensive check when trying to add an item to a dictionary. They only scenarios that could possibly be affected by the fix are currently broken due to this bug.

Eilon · 2017-04-12T17:16:18Z

This patch fix is approved. Please follow the normal code review / pull request process and make sure you make the change in the correct branch.

Eilon · 2017-04-19T18:57:19Z

Preview builds of this patch fix should be available on the following feeds:

https://dotnet.myget.org/gallery/aspnet-1-0-5-may2017-patch-public (for fixes in the 1.0.x release)
- Feed URL to add to NuGet.config: https://dotnet.myget.org/F/aspnet-1-0-5-may2017-patch-public/api/v3/index.json
https://dotnet.myget.org/gallery/aspnet-1-1-2-may2017-patch-public (for fixes in the 1.1.x release)
- Feed URL to add to NuGet.config: https://dotnet.myget.org/F/aspnet-1-1-2-may2017-patch-public/api/v3/index.json

If you have a chance, please try out these preview builds and let us know if you have any feedback!

ajcvickers · 2017-04-24T16:19:58Z

@shaulbehr Could you please check that this issue is fixed for you using the feed posted above? It would be really great to confirm that the issue is fixed in a real-world scenario and that it doesn't regress functionality in some other way. It would be greatly appreciated.

shaulbehr · 2017-04-25T10:12:22Z

@ajcvickers : For our actual production code we decided only to use release code (not previews). But for the sake of this exercise I branched off and upgraded to preview 24623 (which was latest available at this time of writing). Regrettably I couldn't get past my integration test setup because of some other changes that appear to be broken by the way I'm declaring my relationships:

public class UserGroupOrganization
{
    // obviously there's other schema info here...
    public virtual ICollection<UserGroupOrganizationCrossReference> ParentOrganizations { get; set; }
    public virtual ICollection<UserGroupOrganizationCrossReference> ChildOrganizations { get; set; }
}

public class UserGroupOrganizationCrossReference
{
    // other stuff
    public Guid ChildUserGroupOrganizationId { get; set; }
    public Guid ParentUserGroupOrganizationId { get; set; }

    public virtual UserGroupOrganization ChildOrganization { get; set; }
    public virtual UserGroupOrganization ParentOrganization { get; set; }
}

// and inside OnModelCreating():
	modelBuilder.Entity<UserGroupOrganization>(entity =>
	{
		// other stuff
		entity.HasMany(o => o.ParentOrganizations)
			  .WithOne(p => p.ChildOrganization)
			  .HasForeignKey(p => p.ChildUserGroupOrganizationId);
		entity.HasMany(o => o.ChildOrganizations)
			  .WithOne(c => c.ParentOrganization)
			  .HasForeignKey(c => c.ParentUserGroupOrganizationId);
	});

	modelBuilder.Entity<UserGroupOrganizationCrossReference>(entity =>
	{
		// other stuff
		entity.HasOne(x => x.ParentOrganization)
			  .WithMany(o => o.ChildOrganizations)
			  .HasForeignKey(x => x.ParentUserGroupOrganizationId);

		entity.HasOne(x => x.ChildOrganization)
			  .WithMany(o => o.ParentOrganizations)
			  .HasForeignKey(x => x.ChildUserGroupOrganizationId);
	});

Firstly, I know I shouldn't have to declare those relationships in both entities, but I've tried all combinations. And I still get the following error:

System.InvalidOperationException : Unable to determine the relationship represented by navigation property 'UserGroupOrganization.ParentOrganizations' of type 'ICollection'. Either manually configure the relationship, or ignore this property using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

Should I log a new bug for this?

ajcvickers · 2017-04-25T16:01:19Z

@shaulbehr Thanks for trying it. Yes, please file an issue for the new bug. We will investigate.

ajcvickers · 2017-04-25T16:05:52Z

@shaulbehr How are the primary keys defined on those two entity types?

ajcvickers · 2017-04-25T16:12:44Z

@AndriySvyryd Can you investigate this as a possible regression from 1.1.1 to 1.1.2?

ajcvickers · 2017-04-25T16:43:25Z

@shaulbehr Did you test this with the latest nightly build, or with the 1.1.2 preview feed posted in the comments above?

AndriySvyryd · 2017-04-25T18:09:15Z

@shaulbehr I wasn't able to repro the new issue using the 1.1.2 nightly. Please provide a complete code listing when filing a new issue for this.

shaulbehr · 2017-04-26T06:36:30Z

@ajcvickers It was the daily build. I'll try again with the 1.1.2 preview.

ErikEJ · 2017-04-26T08:06:41Z

You made a typo, I think.

https://dotnet.myget.org/gallery/aspnet-1-0-5-may2017-patch-publics

Should be:

https://dotnet.myget.org/F/aspnet-1-0-5-may2017-patch-public/api/v3/index.json

(and public, not publics)

shaulbehr · 2017-04-26T08:27:16Z

@ErikEJ I deleted my previous comment - was my bad.

shaulbehr · 2017-04-26T08:52:19Z

Confirmed: patch has fixed my problem. Thanks!

ajcvickers · 2017-04-26T15:56:43Z

@shaulbehr Thanks for checking! Much appreciated. Can you still file the issue for what you are seeing on dev?

shaulbehr · 2017-04-27T07:29:49Z

@ajcvickers Done
#8302

analogrelay · 2017-04-28T16:35:26Z

Verified fixed in 1.1.2 candidate build.

maumar self-assigned this Mar 21, 2017

maumar added this to the 1.1.2 milestone Mar 21, 2017

maumar added the type-bug label Mar 21, 2017

smitpatel removed this from the 1.1.2 milestone Mar 22, 2017

maumar changed the title ~~EF Core 1.1 has introduced a new ArgumentException bug~~ EF Core 1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model Mar 22, 2017

maumar added this to the 1.1.2 milestone Mar 22, 2017

maumar added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. Servicing-consider labels Mar 23, 2017

Eilon added patch-approved and removed Servicing-consider labels Apr 12, 2017

maumar closed this as completed Apr 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EF Core 1.1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model #7944

EF Core 1.1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model #7944

shaulbehr commented Mar 21, 2017 •

edited

Loading

smitpatel commented Mar 21, 2017

maumar commented Mar 21, 2017

smitpatel commented Mar 21, 2017

maumar commented Mar 22, 2017 •

edited

Loading

maumar commented Mar 22, 2017

maumar commented Mar 23, 2017

maumar commented Mar 23, 2017

Eilon commented Apr 12, 2017

Eilon commented Apr 19, 2017

ajcvickers commented Apr 24, 2017

shaulbehr commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

AndriySvyryd commented Apr 25, 2017

shaulbehr commented Apr 26, 2017

ErikEJ commented Apr 26, 2017

shaulbehr commented Apr 26, 2017

shaulbehr commented Apr 26, 2017

ajcvickers commented Apr 26, 2017

shaulbehr commented Apr 27, 2017

analogrelay commented Apr 28, 2017

EF Core 1.1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model #7944

EF Core 1.1.1 has introduced a new ArgumentException for complex queries with with subqueries, if the same subquery is present multiple times in the query model #7944

Comments

shaulbehr commented Mar 21, 2017 • edited Loading

smitpatel commented Mar 21, 2017

maumar commented Mar 21, 2017

smitpatel commented Mar 21, 2017

maumar commented Mar 22, 2017 • edited Loading

maumar commented Mar 22, 2017

maumar commented Mar 23, 2017

maumar commented Mar 23, 2017

Eilon commented Apr 12, 2017

Eilon commented Apr 19, 2017

ajcvickers commented Apr 24, 2017

shaulbehr commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

ajcvickers commented Apr 25, 2017

AndriySvyryd commented Apr 25, 2017

shaulbehr commented Apr 26, 2017

ErikEJ commented Apr 26, 2017

shaulbehr commented Apr 26, 2017

shaulbehr commented Apr 26, 2017

ajcvickers commented Apr 26, 2017

shaulbehr commented Apr 27, 2017

analogrelay commented Apr 28, 2017

shaulbehr commented Mar 21, 2017 •

edited

Loading

maumar commented Mar 22, 2017 •

edited

Loading