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

[release/7.0] Query: Don't fail translation of aggregate when owned navigations #29243

Merged
merged 1 commit into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -85,6 +87,7 @@ public RelationalSqlTranslatingExpressionVisitor(
_model = queryCompilationContext.Model;
_queryableMethodTranslatingExpressionVisitor = queryableMethodTranslatingExpressionVisitor;
_sqlTypeMappingVerifyingExpressionVisitor = new SqlTypeMappingVerifyingExpressionVisitor();
_throwForNotTranslatedEfProperty = true;
}

/// <summary>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1423,7 +1438,9 @@ private bool TryTranslateAsEnumerableExpression(
MethodInfo method,
List<SqlExpression> scalarArguments)
{
_throwForNotTranslatedEfProperty = false;
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
var selector = TranslateInternal(enumerableExpression.Selector);
_throwForNotTranslatedEfProperty = true;
if (selector != null)
{
enumerableExpression = enumerableExpression.ApplySelector(selector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 19 additions & 0 deletions test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down