From 010b45d851d17634d1ce1195f195a454acace543 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Mon, 26 Aug 2019 18:08:10 -0700 Subject: [PATCH] Query: Bind with members when type has cast to interface Resolves #17276 Resolves #17099 Resolves #16759 --- .../CosmosSqlTranslatingExpressionVisitor.cs | 7 + ...yExpressionTranslatingExpressionVisitor.cs | 5 +- ...lationalSqlTranslatingExpressionVisitor.cs | 5 +- ...ingExpressionVisitor.ExpressionVisitors.cs | 7 +- .../Query/QueryBugsTest.cs | 142 ++++++++++++++++-- 5 files changed, 148 insertions(+), 18 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs index 3a91054a6f3..67eed8538dd 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs @@ -123,6 +123,13 @@ private bool TryBindMember(Expression source, MemberIdentity member, out Express if (source is EntityProjectionExpression entityProjectionExpression) { + if (convertedType != null + && convertedType.IsInterface + && convertedType.IsAssignableFrom(entityProjectionExpression.Type)) + { + convertedType = entityProjectionExpression.Type; + } + expression = member.MemberInfo != null ? entityProjectionExpression.BindMember(member.MemberInfo, convertedType, clientEval: false, out _) : entityProjectionExpression.BindMember(member.Name, convertedType, clientEval: false, out _); diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs index 796a422445d..260b2ee335a 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs @@ -97,11 +97,12 @@ private Expression BindProperty(Expression source, string propertyName, Type typ if (source is EntityProjectionExpression entityProjection) { var entityType = entityProjection.EntityType; - if (convertedType != null) + if (convertedType != null + && !(convertedType.IsInterface + && convertedType.IsAssignableFrom(entityType.ClrType))) { entityType = entityType.GetRootType().GetDerivedTypesInclusive() .FirstOrDefault(et => et.ClrType == convertedType); - if (entityType == null) { return null; diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index e8c136226f8..52ff36a8c2c 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -207,11 +207,12 @@ private bool TryBindMember(Expression source, MemberIdentity member, out Express if (source is EntityProjectionExpression entityProjectionExpression) { var entityType = entityProjectionExpression.EntityType; - if (convertedType != null) + if (convertedType != null + && !(convertedType.IsInterface + && convertedType.IsAssignableFrom(entityType.ClrType))) { entityType = entityType.GetRootType().GetDerivedTypesInclusive() .FirstOrDefault(et => et.ClrType == convertedType); - if (entityType == null) { return false; diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs index e58dec253d8..84914f41005 100644 --- a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs +++ b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs @@ -103,9 +103,12 @@ private Expression TryExpandNavigation(Expression root, MemberIdentity memberIde if (UnwrapEntityReference(innerExpression) is EntityReference entityReference) { var entityType = entityReference.EntityType; - if (convertedType != null) + if (convertedType != null + && !(convertedType.IsInterface + && convertedType.IsAssignableFrom(entityType.ClrType))) { - entityType = entityType.GetTypesInHierarchy().FirstOrDefault(et => et.ClrType == convertedType); + entityType = entityType.GetTypesInHierarchy() + .FirstOrDefault(et => et.ClrType == convertedType); if (entityType == null) { return null; diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index f22186f9933..8e94b18111a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -3523,12 +3523,7 @@ public void Include_with_order_by_on_interface_key() { using (var context = new MyContext10635(_options)) { - Assert.Equal( - CoreStrings.TranslationFailed("(p) => ((IEntity10635)p).Id"), - Assert.Throws( - () => context.Parents.Include(p => p.Children).OrderBy(p => ((IEntity10635)p).Id).ToList()).Message); - - var query2 = context.Parents.Include(p => p.Children).OrderBy(p => EF.Property(p, "Id")).ToList(); + var query = context.Parents.Include(p => p.Children).OrderBy(p => ((IEntity10635)p).Id).ToList(); AssertSql( @"SELECT [p].[Id], [p].[Name], [c].[Id], [c].[Name], [c].[Parent10635Id], [c].[ParentId] @@ -3546,12 +3541,7 @@ public void Correlated_collection_with_order_by_on_interface_key() { using (var context = new MyContext10635(_options)) { - Assert.Equal( - CoreStrings.TranslationFailed("(p) => ((IEntity10635)p).Id"), - Assert.Throws( - () => context.Parents.OrderBy(p => ((IEntity10635)p).Id).Select(p => p.Children.ToList()).ToList()).Message); - - var query2 = context.Parents.OrderBy(p => EF.Property(p, "Id")).Select(p => p.Children.ToList()).ToList(); + var query = context.Parents.OrderBy(p => ((IEntity10635)p).Id).Select(p => p.Children.ToList()).ToList(); AssertSql( @"SELECT [p].[Id], [c].[Id], [c].[Name], [c].[Parent10635Id], [c].[ParentId] @@ -6330,6 +6320,134 @@ public class EntityWithQueryFilterCycle3 #endregion + #region Bug17276_17099_16759 + + [ConditionalFact] + public virtual void Expression_tree_constructed_via_interface_works_17276() + { + using (CreateDatabase17276()) + { + using (var context = new MyContext17276(_options)) + { + var query = List17276(context.RemovableEntities); + + AssertSql( + @"SELECT [r].[Id], [r].[IsRemoved], [r].[Removed], [r].[RemovedByUser] +FROM [RemovableEntities] AS [r] +WHERE [r].[IsRemoved] <> CAST(1 AS bit)"); + } + } + } + + [ConditionalFact] + public virtual void Expression_tree_constructed_via_interface_for_navigation_works_17099() + { + using (CreateDatabase17276()) + { + using (var context = new MyContext17276(_options)) + { + var query = context.Parents + .Where(p => EF.Property(EF.Property(p, "RemovableEntity"), "IsRemoved")) + .ToList(); + + AssertSql( + @"SELECT [p].[Id], [p].[RemovableEntityId] +FROM [Parents] AS [p] +LEFT JOIN [RemovableEntities] AS [r] ON [p].[RemovableEntityId] = [r].[Id] +WHERE [r].[IsRemoved] = CAST(1 AS bit)"); + } + } + } + + [ConditionalFact] + public virtual void Expression_tree_constructed_via_interface_works_16759() + { + using (CreateDatabase17276()) + { + using (var context = new MyContext17276(_options)) + { + var specification = new Specification17276(1); + var entities = context.Set().Where(specification.Criteria).ToList(); + + AssertSql( + @"@__id_0='1' + +SELECT [p].[Id], [p].[RemovableEntityId] +FROM [Parents] AS [p] +WHERE ([p].[Id] = @__id_0) AND @__id_0 IS NOT NULL"); + } + } + } + + public class MyContext17276 : DbContext + { + public DbSet RemovableEntities { get; set; } + public DbSet Parents { get; set; } + public MyContext17276(DbContextOptions options) : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + } + } + + private SqlServerTestStore CreateDatabase17276() + => CreateTestStore( + () => new MyContext17276(_options), + context => + { + context.SaveChanges(); + + ClearLog(); + }); + + public static List List17276(IQueryable query) where T : IRemovable17276 + { + return query.Where(x => !x.IsRemoved).ToList(); + } + + public interface IRemovable17276 + { + bool IsRemoved { get; set; } + + string RemovedByUser { get; set; } + + DateTime? Removed { get; set; } + } + + public class RemovableEntity17276 : IRemovable17276 + { + public int Id { get; set; } + public bool IsRemoved { get; set; } + public string RemovedByUser { get; set; } + public DateTime? Removed { get; set; } + } + + public class Parent17276 : IHasId17276 + { + public int Id { get; set; } + public RemovableEntity17276 RemovableEntity { get; set; } + } + + public interface IHasId17276 + { + T Id { get; } + } + + public class Specification17276 + where T : IHasId17276 + { + public Expression> Criteria { get; } + + public Specification17276(int id) + { + Criteria = t => t.Id == id; + } + } + + #endregion + private DbContextOptions _options; private SqlServerTestStore CreateTestStore(