Query: Copy client projections when updating SelectExpression

Resolves #26428
dotnet · Nov 9, 2021 · 1881ffc · 1881ffc
1 parent 62dc53f
commit 1881ffc
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 6 deletions.
diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections.Generic;
 using System.Linq.Expressions;
 using Microsoft.EntityFrameworkCore.Query;
@@ -50,8 +51,20 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
         private Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
         {
-            return shapedQueryExpression.Update(
-                Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);
+            if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26428", out var enabled)
+                && enabled)
+            {
+                return shapedQueryExpression.Update(
+                    Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);
+            }
+
+            var selectExpression = shapedQueryExpression.QueryExpression;
+            var updatedSelectExpression = Visit(selectExpression);
+            return updatedSelectExpression != selectExpression
+                ? shapedQueryExpression.Update(updatedSelectExpression,
+                    ReplacingExpressionVisitor.Replace(
+                        selectExpression, updatedSelectExpression, shapedQueryExpression.ShaperExpression))
+                : shapedQueryExpression;
         }
 
         private Expression VisitSelect(SelectExpression selectExpression)

diff --git a/...EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs b/...EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq.Expressions;
@@ -54,8 +55,20 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
         private Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
         {
-            return shapedQueryExpression.Update(
-                Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);
+            if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26428", out var enabled)
+                && enabled)
+            {
+                return shapedQueryExpression.Update(
+                    Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);
+            }
+
+            var selectExpression = shapedQueryExpression.QueryExpression;
+            var updatedSelectExpression = Visit(selectExpression);
+            return updatedSelectExpression != selectExpression
+                ? shapedQueryExpression.Update(updatedSelectExpression,
+                    ReplacingExpressionVisitor.Replace(
+                        selectExpression, updatedSelectExpression, shapedQueryExpression.ShaperExpression))
+                : shapedQueryExpression;
         }
 
         private Expression VisitCase(CaseExpression caseExpression)

diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs
@@ -52,8 +52,7 @@ public override Expression Process(Expression query)
             query = new TableAliasVerifyingExpressionVisitor().Visit(query);
 #endif
             query = new SelectExpressionPruningExpressionVisitor().Visit(query);
-            query =
-                new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls)
+            query = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls)
                     .Visit(query);
             query = new RelationalValueConverterCompensatingExpressionVisitor(RelationalDependencies.SqlExpressionFactory).Visit(query);
 

diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -3550,6 +3550,7 @@ public SelectExpression Update(
                 alias, projections.ToList(), tables.ToList(), newTableReferences, groupBy.ToList(), orderings.ToList())
             {
                 _projectionMapping = projectionMapping,
+                _clientProjections = _clientProjections.ToList(),
                 Predicate = predicate,
                 Having = having,
                 Offset = offset,
@@ -3558,6 +3559,12 @@ public SelectExpression Update(
                 Tags = Tags
             };
 
+            if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26428", out var enabled)
+                && enabled)
+            {
+                newSelectExpression._clientProjections = new();
+            }
+
             // We don't copy identifiers because when we are doing reconstruction so projection is already applied.
             // Update method should not be used pre-projection application. There are other methods to change SelectExpression.
 

diff --git a/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs
@@ -324,5 +324,103 @@ public PhotoBlog()
 
             public int NumberOfPhotos { get; set; }
         }
+
+#nullable enable
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual async Task IsDeleted_query_filter_with_conversion_to_int_works(bool async)
+        {
+            var contextFactory = await InitializeAsync<Context26428>(seed: c => c.Seed());
+            using var context = contextFactory.CreateContext();
+
+            var query = context.Suppliers.Include(s => s.Location).OrderBy(s => s.Name);
+
+            var suppliers = async
+                ? await query.ToListAsync()
+                : query.ToList();
+
+            Assert.Equal(4, suppliers.Count);
+            Assert.Single(suppliers.Where(e => e.Location != null));
+        }
+
+        protected class Context26428 : DbContext
+        {
+            public Context26428(DbContextOptions options)
+                   : base(options)
+            {
+            }
+
+            public DbSet<Supplier> Suppliers => Set<Supplier>();
+            public DbSet<Location> Locations => Set<Location>();
+
+            protected override void OnModelCreating(ModelBuilder modelBuilder)
+            {
+                modelBuilder.Entity<Supplier>().Property(s => s.IsDeleted).HasConversion<int>();
+                modelBuilder.Entity<Supplier>().HasQueryFilter(s => !s.IsDeleted);
+
+                modelBuilder.Entity<Location>().Property(l => l.IsDeleted).HasConversion<int>();
+                modelBuilder.Entity<Location>().HasQueryFilter(l => !l.IsDeleted);
+            }
+
+            public void Seed()
+            {
+                var activeAddress = new Location
+                {
+                    Address = "Active address",
+                    IsDeleted = false
+                };
+                var deletedAddress = new Location
+                {
+                    Address = "Deleted address",
+                    IsDeleted = true
+                };
+
+                var activeSupplier1 = new Supplier
+                {
+                    Name = "Active supplier 1",
+                    IsDeleted = false,
+                    Location = activeAddress
+                };
+                var activeSupplier2 = new Supplier
+                {
+                    Name = "Active supplier 2",
+                    IsDeleted = false,
+                    Location = deletedAddress
+                };
+                var activeSupplier3 = new Supplier
+                {
+                    Name = "Active supplier 3",
+                    IsDeleted = false
+                };
+                var deletedSupplier = new Supplier
+                {
+                    Name = "Deleted supplier",
+                    IsDeleted = false
+                };
+
+                AddRange(activeAddress, deletedAddress);
+                AddRange(activeSupplier1, activeSupplier2, activeSupplier3, deletedSupplier);
+
+                SaveChanges();
+            }
+        }
+
+        protected class Supplier
+        {
+            public Guid SupplierId { get; set; }
+            public string Name { get; set; } = null!;
+            public Location? Location { get; set; }
+            public bool IsDeleted { get; set; }
+        }
+
+        protected class Location
+        {
+            public Guid LocationId { get; set; }
+            public string Address { get; set; } = null!;
+            public bool IsDeleted { get; set; }
+        }
+
+#nullable disable
     }
 }