Query: Don't fail translation of aggregate when owned navigations

Resolves #29201
dotnet · Oct 3, 2022 · a05de75 · a05de75
1 parent a6f905a
commit a05de75
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 2 deletions.
diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
@@ -68,6 +68,8 @@ private static readonly MethodInfo StringEqualsWithStringComparisonStatic
     private readonly QueryableMethodTranslatingExpressionVisitor _queryableMethodTranslatingExpressionVisitor;
     private readonly SqlTypeMappingVerifyingExpressionVisitor _sqlTypeMappingVerifyingExpressionVisitor;
 
+    private bool _throwForNotTranslatedEfProperty;
+
     /// <summary>
     ///     Creates a new instance of the <see cref="RelationalSqlTranslatingExpressionVisitor" /> class.
     /// </summary>
@@ -85,6 +87,7 @@ public RelationalSqlTranslatingExpressionVisitor(
         _model = queryCompilationContext.Model;
         _queryableMethodTranslatingExpressionVisitor = queryableMethodTranslatingExpressionVisitor;
         _sqlTypeMappingVerifyingExpressionVisitor = new SqlTypeMappingVerifyingExpressionVisitor();
+        _throwForNotTranslatedEfProperty = true;
     }
 
     /// <summary>
@@ -723,8 +726,20 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
         // EF.Property case
         if (methodCallExpression.TryGetEFPropertyArguments(out var source, out var propertyName))
         {
-            return TryBindMember(Visit(source), MemberIdentity.Create(propertyName))
-                ?? throw new InvalidOperationException(CoreStrings.QueryUnableToTranslateEFProperty(methodCallExpression.Print()));
+            if (TryBindMember(Visit(source), MemberIdentity.Create(propertyName)) is SqlExpression result)
+            {
+                return result;
+            }
+
+            var message = CoreStrings.QueryUnableToTranslateEFProperty(methodCallExpression.Print());
+            if (_throwForNotTranslatedEfProperty)
+            {
+                throw new InvalidOperationException(message);
+            }
+
+            AddTranslationErrorDetails(message);
+
+            return QueryCompilationContext.NotTranslatedExpression;
         }
 
         // EF Indexer property
@@ -1423,7 +1438,9 @@ private bool TryTranslateAsEnumerableExpression(
         MethodInfo method,
         List<SqlExpression> scalarArguments)
     {
+        _throwForNotTranslatedEfProperty = false;
         var selector = TranslateInternal(enumerableExpression.Selector);
+        _throwForNotTranslatedEfProperty = true;
         if (selector != null)
         {
             enumerableExpression = enumerableExpression.ApplySelector(selector);

diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs
@@ -441,6 +441,14 @@ public override async Task Left_join_on_entity_with_owned_navigations_complex(bo
         AssertSql(" ");
     }
 
+    [ConditionalTheory(Skip = "GroupBy #17314")]
+    public override async Task GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(bool async)
+    {
+        await base.GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(async);
+
+        AssertSql();
+    }
+
     public override async Task Filter_on_indexer_using_closure(bool async)
     {
         await base.Filter_on_indexer_using_closure(async);

diff --git a/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
@@ -881,6 +881,25 @@ from sub in grouping2.DefaultIfEmpty()
                 AssertEqual(e.sub.c2, a.sub.c2);
             });
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(bool async)
+        => AssertQuery(
+            async,
+            ss => ss.Set<OwnedPerson>()
+                    .GroupBy(e => e.Id)
+                    .Select(e => new
+                    {
+                        e.Key,
+                        Sum = e.Sum(i => i.PersonAddress.Country.PlanetId)
+                    }),
+            elementSorter: e => e.Key,
+            elementAsserter: (e, a) =>
+            {
+                AssertEqual(e.Key, a.Key);
+                AssertEqual(e.Sum, a.Sum);
+            });
+
     protected virtual DbContext CreateContext()
         => Fixture.CreateContext();
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedQuerySqlServerTest.cs
@@ -1236,6 +1236,19 @@ FROM [Order] AS [o0]
 ORDER BY [p].[Id], [t].[Id], [t].[Id0], [t0].[ClientId], [t0].[Id], [t0].[OrderClientId], [t0].[OrderId]");
     }
 
+    public override async Task GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(bool async)
+    {
+        await base.GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(async);
+
+        AssertSql(
+            @"SELECT [o].[Id] AS [Key], (
+    SELECT COALESCE(SUM([o0].[PersonAddress_Country_PlanetId]), 0)
+    FROM [OwnedPerson] AS [o0]
+    WHERE [o].[Id] = [o0].[Id]) AS [Sum]
+FROM [OwnedPerson] AS [o]
+GROUP BY [o].[Id]");
+    }
+
     private void AssertSql(params string[] expected)
         => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalOwnedQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalOwnedQuerySqlServerTest.cs
@@ -1224,6 +1224,19 @@ LEFT JOIN (
 ORDER BY [p].[Id], [t].[Id], [t].[Id0], [t0].[ClientId], [t0].[Id], [t0].[OrderClientId], [t0].[OrderId]");
     }
 
+    public override async Task GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(bool async)
+    {
+        await base.GroupBy_aggregate_on_owned_navigation_in_aggregate_selector(async);
+
+        AssertSql(
+            @"SELECT [o].[Id] AS [Key], (
+    SELECT COALESCE(SUM([o0].[PersonAddress_Country_PlanetId]), 0)
+    FROM [OwnedPerson] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [o0]
+    WHERE [o].[Id] = [o0].[Id]) AS [Sum]
+FROM [OwnedPerson] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [o]
+GROUP BY [o].[Id]");
+    }
+
     // not AssertQuery so original (non-temporal) query gets executed, but data is modified
     // so results don't match expectations
     public override Task Owned_entity_without_owner_does_not_throw_for_identity_resolution(bool async, bool useAsTracking)