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

QueryRewrite: support contains on collection navigation #15554

Closed
maumar opened this issue May 1, 2019 · 7 comments · Fixed by #16841
Closed

QueryRewrite: support contains on collection navigation #15554

maumar opened this issue May 1, 2019 · 7 comments · Fixed by #16841
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented May 1, 2019

query:

customers.Select(c => c.Orders.Contains(someOrder))

currently Enumerable.Contains is translated to IN, which is incorrect for the case above

@ajcvickers ajcvickers added this to the 3.0.0 milestone May 3, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview7 Jul 2, 2019
@ajcvickers
Copy link
Member

@maumar Why is the translation to IN incorrect?

@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jul 23, 2019
@divega
Copy link
Contributor

divega commented Jul 24, 2019

@ajcvickers, @smitpatel I tried this to try to answer the question above, and a query like this throws before generating any SQL using nightly builds. Is this a known issue:

System.InvalidOperationException
  HResult=0x80131509
  Message=Operation is not valid due to the current state of the object.
  Source=Microsoft.EntityFrameworkCore.Relational
  StackTrace:
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateSubquery(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalSqlTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.Visit(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalProjectionBindingExpressionVisitor.Translate(SelectExpression selectExpression, Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.TranslateSelect(ShapedQueryExpression source, LambdaExpression selector)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQueryCore[TFunc](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Repro14173.Program.Main(String[] args) in C:\Users\divega\source\repos\Repro14173\Program.cs:line 21

@divega divega removed this from the Backlog milestone Jul 24, 2019
@smitpatel
Copy link
Contributor

It should have been rewritten by #15939

Can you write PK based equality directly and see if it translates?

@divega
Copy link
Contributor

divega commented Jul 24, 2019

@smitpatel yes, it does translate:

SELECT CASE
    WHEN @__someThing_Id_0 IN (
        SELECT [t].[Id]
        FROM [Things] AS [t]
        WHERE ([p].[Id] = [t].[PersonId]) AND [t].[PersonId] IS NOT NULL
    )
     THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END
FROM [People] AS [p]

Repro:

using System;
using System.Collections.Generic;
using System.Data.Common;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;

namespace Repro15554
{
    class Program
    {
        static void Main(string[] args)
        {
            using (var context = new MyContext())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();
                var someThing = new Thing { Id = 1 };

                var query1 = context.People
                    .Select(p => p.Things.Contains(someThing))
                    .ToList(); // fails

                var query2 = context.People
                    .Select(p => p.Things.Select(t => t.Id).Contains(someThing.Id))
                    .ToList(); // works
            }
        }
    }

    public class MyContext : DbContext
    {
        public DbSet<Person> People { get; set; }

        public DbSet<Thing> Things { get; set; }
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .UseSqlServer(@"server=(localdb)\mssqllocaldb; database=parameternames; integrated security=true; connectretrycount=0")
                .AddInterceptors(new LoggingInterceptor(Console.WriteLine));
        }
    }

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

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

        public ICollection<Thing> Things { get; set; } = new List<Thing>();
    }

    public class LoggingInterceptor : DbCommandInterceptor
    {
        private readonly Action<string> _loggingAction;
        public LoggingInterceptor(Action<string> loggingAction)
        {
            _loggingAction = loggingAction;
        }
        public override InterceptionResult<DbDataReader> ReaderExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<DbDataReader> result)
        {
            _loggingAction(command.CommandText);
            return base.ReaderExecuting(command, eventData, result);
        }
    }
}

@smitpatel
Copy link
Contributor

So now the questions:

  • Is translation of second query above incorrect?
    • If yes, then why? and what should we translate it to?
    • If no, then first query should be converted to second by entity equality rewrite.

@divega
Copy link
Contributor

divega commented Jul 24, 2019

The translation looks correct to me 😄

@maumar
Copy link
Contributor Author

maumar commented Jul 29, 2019

Contains on keys seems correct to me also. I don't remember what I meant by my initial comment, perhaps we tried to create IN directly on entity objects, rather than keys. Either way, I agree with @smitpatel and @divega here

roji added a commit that referenced this issue Jul 30, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Jul 30, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 30, 2019
roji added a commit that referenced this issue Jul 30, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Jul 30, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 1, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 2, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 2, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 2, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 2, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 3, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 4, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
roji added a commit that referenced this issue Aug 4, 2019
EE handled the extension version of {IQueryable,IEnumerable}.Contains,
but not instance methods such as List.Contains.

Fixes #15554
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview9 Aug 21, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview9, 3.0.0 Nov 11, 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. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants